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 the dependencies to GCS, S3 and Azure Storage with extras_require #12

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

yu-iskw
Copy link
Contributor

@yu-iskw yu-iskw commented Jan 31, 2019

@mouradmourafiq

Overview

I put the extra dependencies to GCS, S3 and Azure Storage, since extras_require enables us to controll the installation to keep it lightweight.

Comments

I haven't made sure if the modified client works or not with google cloud storage, s3 and azure storage yet. When I have time, I would like to do that with GCS. But since I am not a sophisticated AWS and Azure user, I won't be able to test it.

Besides, can you guide me if it has to depends on specific versions of the modules respectively?
For instance,

...
   'gcs': ['google-cloud-storage>=1.13.0'],
...

Issue

polyaxon/polyaxon#352

@yu-iskw yu-iskw changed the title Add the dependencies to GCS, S3 and Azure Storage with extras_requires. Add the dependencies to GCS, S3 and Azure Storage with extras_require Jan 31, 2019
@mmourafiq
Copy link
Collaborator

I think that we can keep it as it is now, if someone reports a bug related to a specific version, we can then either set stronger dependencies or make the package compatible.

Copy link
Collaborator

@mmourafiq mmourafiq left a comment

Choose a reason for hiding this comment

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

I will merge as soon as you let me know that you have tested that the change works with GCS.

I will test the usage with Azure and S3, and possibly CGS, if I manage to test it before, I will merge the PR as well.

Thanks.

@mmourafiq
Copy link
Collaborator

Tried it both on azure and gcs, will try S3 later, but looks good, in any case, it's not a blocker, users can always pip install what they want.

Thanks and Sorry for late merge.

@mmourafiq mmourafiq merged commit 480a5a9 into polyaxon:master Feb 11, 2019
@yu-iskw
Copy link
Contributor Author

yu-iskw commented Feb 11, 2019

Sorry for the delay of making sure of it. And thank you for merging it.

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