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

fix: Propagate command line arguments #35

Merged
merged 2 commits into from
May 13, 2017

Conversation

lfiaschi
Copy link
Contributor

The command line arguments were not correctly propagated, which do not allowed using flags as
--noauth_local_webserve. This seems to fix the issue and now the flag is correctly passed downstream.

@codecov-io
Copy link

codecov-io commented May 10, 2017

Codecov Report

Merging #35 into master will decrease coverage by 44.4%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #35       +/-   ##
===========================================
- Coverage   73.72%   29.31%   -44.41%     
===========================================
  Files           4        4               
  Lines        1511     1511               
===========================================
- Hits         1114      443      -671     
- Misses        397     1068      +671
Impacted Files Coverage Δ
pandas_gbq/gbq.py 16.08% <0%> (-62.12%) ⬇️
pandas_gbq/tests/test_gbq.py 31.88% <0%> (-49.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dfd106...44aa7e8. Read the comment docs.

@lfiaschi
Copy link
Contributor Author

fixes #36

@jreback
Copy link
Contributor

jreback commented May 10, 2017

an you write a test which fails before, but now passes?

@lfiaschi
Copy link
Contributor Author

lfiaschi commented May 10, 2017

unfortunately, it is quite difficult to test this, because it's a command line feature which requires human interaction (auth flow).

Briefly, it only happens when the file bigquery_credentials.dat is not present. Then oauthlib will ask the user to login into a certain url with a gmail address. The type of url and other inputs that the user has to enter manually depend on the flags which are passed in the main process.

I am not sure of the best way to test this interaction.

@jreback
Copy link
Contributor

jreback commented May 10, 2017

ok can u add an entry in the changelog.rst
add a new entry for 0.1.7 (create the section)

@jreback
Copy link
Contributor

jreback commented May 10, 2017

@parthea

@jreback jreback modified the milestone: 0.1.7 May 12, 2017
@jreback jreback added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label May 12, 2017
@parthea parthea force-pushed the fix/parse-arguments branch from fdc763b to 44aa7e8 Compare May 13, 2017 15:28
@parthea
Copy link
Contributor

parthea commented May 13, 2017

This worked as expected in my limited testing locally. Without the fix, I received the following message with --noauth_local_webserver:

Your browser has been opened to visit:
....
If your browser is on a different machine then exit and re-run this
application with the command-line parameter 

  --noauth_local_webserver

Here is my output with the fix. Notice that the message changed from "Your browser has been opened to visit:" to "Go to the following link in your browser:"

tony@tonypc:~/pydata-pandas-gbq$ python
Python 2.7.12 |Continuum Analytics, Inc.| (default, Jul  2 2016, 17:42:40) 
[GCC 4.4.7 20120313 (Red Hat 4.4.7-1)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
Anaconda is brought to you by Continuum Analytics.
Please check out: http://continuum.io/thanks and https://anaconda.org
>>> import sys
>>> sys.argv=['gbq.py','--noauth_local_webserver']
>>> import pandas_gbq as p
>>> p.gbq.GbqConnector(<your_project_id>)
Go to the following link in your browser:

Copy link
Contributor

@parthea parthea left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @lfiaschi !

@parthea parthea merged commit be18920 into googleapis:master May 13, 2017
@jreback jreback modified the milestones: 0.1.7, 0.2.0 May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants