Skip to content
This repository was archived by the owner on Mar 13, 2026. It is now read-only.

fix: Propagate command line arguments#35

Merged
parthea merged 2 commits into
googleapis:masterfrom
lfiaschi:fix/parse-arguments
May 13, 2017
Merged

fix: Propagate command line arguments#35
parthea merged 2 commits into
googleapis:masterfrom
lfiaschi:fix/parse-arguments

Conversation

@lfiaschi

Copy link
Copy Markdown
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

codecov-io commented May 10, 2017

Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

fixes #36

@jreback

jreback commented May 10, 2017

Copy link
Copy Markdown
Contributor

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

@lfiaschi

lfiaschi commented May 10, 2017

Copy link
Copy Markdown
Contributor Author

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

jreback commented May 10, 2017

Copy link
Copy Markdown
Contributor

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

@jreback

jreback commented May 10, 2017

Copy link
Copy Markdown
Contributor

@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

parthea commented May 13, 2017

Copy link
Copy Markdown
Contributor

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:

@parthea parthea left a comment

Copy link
Copy Markdown
Contributor

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

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