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

Construct CartAPI URL from OS environment variables #15

Merged
merged 14 commits into from
Mar 6, 2019

Conversation

markborkum
Copy link
Contributor

Description

This pull request modifies the constructors for the Downloader and CartAPI classes so that CartAPI URLs are automatically constructed from either user-specified keyword arguments or OS environment variables.

The intent is that the type signature for the constructor for the Downloader class should resemble that of the Uploader (e.g., use keyword arguments to specify and/or construct URLs).

Changes:

Issues Resolved

n/a

Check List

@markborkum
Copy link
Contributor Author

@dmlb2000 In https://github.com/pacifica/pacifica-python-downloader/pull/15/files#diff-d8e165b4149566ddfdab844d646f4d6fR51, the prefix for environment variables for the CartAPI class is "CART". Is this correct?

@dmlb2000
Copy link
Member

dmlb2000 commented Mar 6, 2019

@markborkum We've been standardizing on CARTD

@markborkum
Copy link
Contributor Author

@dmlb2000 Also, should we take the opportunity to add more LOGGER.debug(...) statements to the CartAPI class (e.g., the setup_cart and wait_for_cart instance methods)?

@dmlb2000
Copy link
Member

dmlb2000 commented Mar 6, 2019

@markborkum Yes more logging the better really.

@markborkum
Copy link
Contributor Author

@dmlb2000 CI is failing due to autopep8. Any ideas?

@dmlb2000
Copy link
Member

dmlb2000 commented Mar 6, 2019

@markborkum Yes, autopep8 is kinda lame, I would look for long lines and split them up...

foo(a, b, c, d, e)

becomes

foo(
    a,
    b,
    c,
    d,
    e,
)

@markborkum
Copy link
Contributor Author

@dmlb2000 Ready for review.

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.

None yet

2 participants