-
Notifications
You must be signed in to change notification settings - Fork 51
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
base: main
Are you sure you want to change the base?
Conversation
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. |
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, |
There was a problem hiding this 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'])], |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
Creates docker images with tags: