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

Add automatic docker image building #296

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

almahmoud
Copy link
Collaborator

image
Creates docker images with tags:

3.9-aws
3.9-azure
3.9-full
3.9-gcp
3.9-openstack
dev-3.9-aws
dev-3.9-azure
dev-3.9-full
dev-3.9-gcp
dev-3.9-openstack

@nuwang
Copy link
Contributor

nuwang commented Feb 3, 2022

What's the expected use case for this? To be able to run a quick ad-hoc script? If so, I'd trim this down to just one image with all providers installed.

@almahmoud
Copy link
Collaborator Author

My immediate usecase is making Github actions that push to SWIFT object store. I'm fine collapsing to have full across the board, I would highly suggest leaving dev vs slim though, dev is helpful for trying things out manually, but size of slim is ~8x smaller. More generally, if this doesn't seem desirable as something to maintain, also happy to leave this just in fork as well.

Copy link
Contributor

@nuwang nuwang left a comment

Choose a reason for hiding this comment

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

I think we can merge this, but shall we then slim this down to just 'dev' and 'slim'? I think that fewer is better in this instance.

Also, we may be able to get rid of slim too if the docker image is cached on github?

parser.add_argument('destination', help="Destination in bucket", type=str)
parser.add_argument('-b', '--bucket', help="Name of destination bucket", type=str)
parser.add_argument('-p', '--provider',
choices=[ProviderList.__dict__[p] for p in ProviderList.__dict__.keys() if all(x not in p for x in ['_', 'MOCK'])],
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it's time we make ProviderList an enum? https://stackoverflow.com/questions/29503339/how-to-get-all-values-from-python-enum-class/29503414

I think this is just a legacy thing from python 2 days.

count = 0
while not uploaded:
print(f'Trying for the {count}th time')
uploaded = ob.upload_from_file(args.filepath)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to have exposed another bug in the API. Not all implementations return a bool here, only openstack and azure do. I think the correct behaviour should be to raise an exception if the upload fails, instead of returning a bool value.

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.

2 participants