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

Docker optional dependency #28

Merged
merged 3 commits into from
Jun 6, 2022
Merged

Docker optional dependency #28

merged 3 commits into from
Jun 6, 2022

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented May 27, 2022

We currently are just using it for login and finding configs, and since osx on conda does not have it (and we want to support it) it makes sense to have docker be optional, meaning we fall back to manual ways to do the same thing. I am leaving it as an extra variant for now in case someone prefers using docker, however if we have confidence it is not needed it can be removed entirely.

See conda-forge/oras-py-feedstock#1 and ping @wolfv

we currently are just using it for login and finding configs, and since
osx on conda does not have it (and we want to support it) it makes sense
to have docker be optional, meaning we fall back to manual ways to
do the same thing. I am leaving it as an extra variant for now in case
someone prefers using docker, however if we have confidence it is not
needed it can be removed entirely.

Signed-off-by: vsoch <[email protected]>
@vsoch vsoch force-pushed the docker-optional-dependency branch from 6f24938 to 524e22c Compare May 27, 2022 17:14
@wolfv
Copy link
Contributor

wolfv commented May 27, 2022

Awesome! I think the regular OS X 64 on conda-forge has it, just the osx-arm64 (M1) chip not yet.

@vsoch
Copy link
Contributor Author

vsoch commented May 27, 2022

Ah gotcha! Also I was testing newer versions of docker (on Linux/Ubuntu) and the function behavior wasn't consistent (e.g., updating broke some functions) so that is what first incentivized me to want to remove it / make it optional. Likely we will need a user that has a more complicated docker config setup to know if/what is missing.

Copy link

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@vsoch
Copy link
Contributor Author

vsoch commented Jun 5, 2022

Thank you! I think we will need an admin approval (ping @sajayantony!) before the merge is allowed (and I'm not too keen on clicking the "merge without waiting for requirements to be met" box.

When this goes in, I'm planning to release after #32 is merged since it fixes some bugs with respect to pushing to GitHub packages. Then we will have a new release for conda-forge too!

Signed-off-by: vsoch <[email protected]>
@vsoch vsoch merged commit abcc5d5 into main Jun 6, 2022
@vsoch vsoch deleted the docker-optional-dependency branch June 6, 2022 20:57
@vsoch
Copy link
Contributor Author

vsoch commented Jun 8, 2022

@wolfv I've just triggered a release, so I'll follow up on conda-forge to update the recipe to not require docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants