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

Fixed one test and used an env var for the google cloud bucket #241

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

EtienneBerube
Copy link

The bucket name will be given as an environment variable as the method to_text() is only given one argument when used in extract_data().

The fixed test was using the wrong file extension to compare the results. Therefore, the test would not pass.

The bucket name will be given as an environment variable as the method to_text() is only given one argument when used in extract_data
@m3nu
Copy link
Collaborator

m3nu commented Jul 11, 2019

I don't think we want to remove the function argument altogether.

@EtienneBerube
Copy link
Author

Note token. I removed the "default" option of the Google Cloud bucket name as it needs to be changed anyway by a new user (now defaults to None). Now, it can either be an argument, or an environment variable (depends on application). Can now be in a docker.

Copy link

@rakshithrs rakshithrs left a comment

Choose a reason for hiding this comment

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

Yes, env var is required for google cloud bucket

Copy link

@rakshithrs rakshithrs left a comment

Choose a reason for hiding this comment

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

env var is required for google cloud bucket

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

Successfully merging this pull request may close these issues.

3 participants