Rewriting bigquery samples to use proper argparse and main execution

[theacodes]

Resolves #80.

[elibixby]

Just to give my thinking behind this. Camel case matches the field names in the json objects. So you could make these kwargs and then do something like:

tableRef = fetch_some_bigquery_object()['tableReference']
export_table(service, cloud_storage, **tableRef)

Probably not worth inconsistent/camel case naming, but something to consider.

[theacodes]

True, but I'd rather follow python convention where possible:

Names that are visible to the user as public parts of the API should follow conventions that reflect usage rather than implementation.
...
mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

[theacodes]

/cc @jerjou

[theacodes]

gotta figure out what's up with the tests.

[jerjou]

I like.
LGTM after tests pass, and Eli gives his stamp of approval.

[theacodes]

@elibixby, @jerjou so the issue with tests is murky.

We're expecting users to run these scripts as such:

python bigquery/samples/async_query.py [...]

However, this causes the file to have the __name__ of "__main__" which means that relative imports do not work, and importing sibling modules only works because of some slightly sneaky behavior. So, right now I have

import utils

in the bigquery samples, which works if you execute the script directly (because it's a sibling module), but breaks when the tests import this script because it's expected a top-level module named utils.

The correct way to fix this is to change the way users execute the file to:

python -m bigquery.samples.async_query [args]

Is this acceptable for you guys? I can't come up with very many other clever ways of doing things without black magic.

[theacodes]

This is a recurring problem in a couple of samples. It appears we've been making it impossible for users to execute these the normal way. We have two options here:

1. Tell users to use python -m to run the samples. We can do this in top-level readmes.
2. Remove dependent modules and make all sample free-standing. So bigquery.utils would go away and get_service, poll_job, etc. would be moved into individual files.

I don't have a horse in this race. But I have two conflicting feelings:

• I don't like code duplication.
• These are samples and each sample should be self-contained.

/cc @jerjou @elibixby @tmatsuo @SurferJeffAtGoogle for voting.

[elibixby]

My vote goes to python -m. Looking at the docs I don't think this flag is terribly opaque, and I think it's also good for users to have one place to look for helpful utils

[theacodes]

Will wait for the remaining cc'd folks to cast their vote before deciding.

[theacodes]

/cc @waprin too for voting.

[tmatsuo]

Does the following work:

if __name__ == '__main__' and __package__ is None:
    import bigquery.samples
    __package__ = 'bigquery.samples'

from . import utils

? Although it seems hacky.

I'm fine with -m option.

[jerjou]

I am conflicted as well. It does seem the python -m flag is the more maintainable of the two, but it seems we'd have to audit our docs to make sure our instructions include it.

...OTOH, these are samples and they'd probably have nominally more utility as such if they were self-contained. And the functions in question - at least here - aren't that many lines of code, so duplication wouldn't be that bad..

I think I'm leaning more towards self-contained samples, but I'd really be fine either way (well, maybe not fine, but about the same degree of resigned)

[theacodes]

Ha, @jerjou make a decisive vote!

@tmatsuo I'd rather not have hacky code at the top of our samples just for our benefit.

These samples are for our users, and tests are there to ensure things work for our users - which is why I'm okay with code duplication as long as our tests are good. If duplicating code makes testing easier and makes it easier for users to run our samples, then I think that's a win.

[tmatsuo]

I'm fine with either of them, but I'm still thinking there should be a way to handle this (correctly and not a hacky way). Maybe I'm wrong.

[theacodes]

Afaik, there isn't. This is just a quirk of python and it hurts in our case. Most "executable" scripts should just add a simple bootstrap script to their distribution:

from somepackage import run

if __name__=='__main__':
    run()

See psq for a slightly more advanced example, but same principle.

I'm personally slightly more in favor of dropping utils and improving test coverage to ensure that they still work.

[jerjou]

I vote for self-contained - it makes for better samples, when you can have confidence that what you're seeing is all you need to write.

[tmatsuo]

If I had to choose one of them, I choose dups.

[waprin]

I vote for -m. It seems designed for this exact purpose and so is the most Pythonic, even if Python is occasionally super hacky. I don't like code duplication from a maintenance standpoint. I don't think it's critical that samples are self-contained, as long as they are relatively easy to copy/paste and get working. I don't mind either way, but my decisive vote is -m.

[theacodes]

My vote is for duplicating these handful of functions. Our users should not have to understand our repository or workflow to use our samples.

For duplication: jerjou, tmatsuo, jonwayne
For -m: elibixby, waprin

I'll start down the dup path, but if anyone else votes I can undo it.