Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable code that depends on conda-docker #667

Merged
merged 6 commits into from
Nov 22, 2023

Conversation

nkaretnikov
Copy link
Contributor

See #666.

Copy link

netlify bot commented Nov 21, 2023

Deploy Preview for kaleidoscopic-dango-0cf31d canceled.

Name Link
🔨 Latest commit a566e7d
🔍 Latest deploy log https://app.netlify.com/sites/kaleidoscopic-dango-0cf31d/deploys/655ddf3bc09e520008e1d4d6

@nkaretnikov nkaretnikov force-pushed the skip-conda-docker-test branch from 53b26fe to 65fc72e Compare November 21, 2023 09:11
@nkaretnikov nkaretnikov force-pushed the skip-conda-docker-test branch from e524bb5 to bf69e05 Compare November 21, 2023 10:32
@nkaretnikov nkaretnikov changed the title Skip test_generate_conda_docker Disable action_generate_conda_docker on all platforms Nov 21, 2023
@nkaretnikov nkaretnikov changed the title Disable action_generate_conda_docker on all platforms Disable code that depends on conda-docker Nov 21, 2023
@nkaretnikov nkaretnikov marked this pull request as ready for review November 21, 2023 16:25
@costrouc
Copy link
Member

costrouc commented Nov 21, 2023

The conda-store docker bits are not well maintained at this moment. I would be fine with merging this but would like us to have a plan for if in the future we would like to support this. This feature is powerful since it allows conda-store to build docker images of environments without docker/root but I would say was largely an experiment.

Copy link
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a warning saying "Temporarily disabled; see XXX" and return early instead of commenting things out? Thanks!

@nkaretnikov
Copy link
Contributor Author

@costrouc I'm planning to fix the upstream issue in conda-docker ASAP. There are some suggestions from Jaime to try, but I cannot say anything concrete since I haven't tried anything yet.

@jaimergp Updated. I've used the same message everywhere that we have elsewhere instead of the "Temporary" one. I also don't like the word "temporary" since it's somewhat meaningless (everything is temporary to some extent). I think the other message conveys the intention to support this feature in the future as well. Using the same message in the warning and error will also make it easier to discover both once people search for one of them. I also use a full URL instead of an issue number to make it easier to understand which repo this refers to.

@nkaretnikov nkaretnikov requested a review from jaimergp November 22, 2023 11:21
@nkaretnikov nkaretnikov merged commit 53d224e into conda-incubator:main Nov 22, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

3 participants