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

allow hostnames to be specified via environment variables #102

Merged
merged 6 commits into from
Jul 3, 2019
Merged

Conversation

bryonjacob
Copy link
Contributor

This PR allows for directing this SDK to a different set of URLs from the default set used at https://data.world

Copy link
Contributor

@laconc laconc left a comment

Choose a reason for hiding this comment

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

This is great. Perhaps, we could also add a snippet concerning this to the readme?

datadotworld/client/api.py Outdated Show resolved Hide resolved
Copy link
Contributor

@laconc laconc left a comment

Choose a reason for hiding this comment

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

Love what you're doing here, @kndungu!

I did notice some flake8 issues that are preventing the checks from passing, specifically:

./datadotworld/files.py:31:1: E302 expected 2 blank lines, found 1
./datadotworld/client/api.py:41:1: E302 expected 2 blank lines, found 1
./datadotworld/hosts/__init__.py:28:1: E302 expected 2 blank lines, found 1
./datadotworld/hosts/__init__.py:29:3: E111 indentation is not a multiple of four
./datadotworld/hosts/__init__.py:30:3: E111 indentation is not a multiple of four
./datadotworld/hosts/__init__.py:31:3: E111 indentation is not a multiple of four
./datadotworld/hosts/__init__.py:34:3: E111 indentation is not a multiple of four
./datadotworld/hosts/__init__.py:36:1: E305 expected 2 blank lines after class or function definition, found 1
./datadotworld/hosts/__init__.py:37:80: E501 line too long (91 > 79 characters)
./datadotworld/hosts/__init__.py:38:80: E501 line too long (106 > 79 characters)
./datadotworld/hosts/__init__.py:39:80: E501 line too long (97 > 79 characters)

Thanks for taking the time to finish up this PR!

return endpoint

DW_ENVIRONMENT = environ.get('DW_ENVIRONMENT', '')
API_HOST = environ.get('DW_API_HOST', create_url('https://api.data.world', DW_ENVIRONMENT))
Copy link
Contributor

Choose a reason for hiding this comment

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

For this, what do you think about rewriting create_url to just require the subdomain? So this line would look like:

API_HOST = environ.get('DW_API_HOST', create_url('api', DW_ENVIRONMENT))

And then create_url could be something simpler like:

def create_url(subdomain, environment):
    if environment:
        subdomain = subdomain + '.' + environment

    return 'https://{}.data.world'.format(subdomain)

@kndungu
Copy link

kndungu commented Jul 3, 2019

My pleasure @laconc!

Issues fixed and suggestion applied.

@laconc laconc merged commit 2dad657 into master Jul 3, 2019
@laconc laconc deleted the bryon branch July 3, 2019 19:19
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