Flesh out bigquery API

[tseaver]

• Add missing top-level convenience imports
• Add missing Client.list_datasets and Dataset.list_tables methods.

[dhermes]

Really easy to review! Thanks.

Only a few hangups, pubsub in docstrings, KeyError questions in factories and use of ? in a docstring.

[tseaver]

@dhermes I think everything is resolved, except the question about KeyErrors for missing datasetReference / tableReference entries: my take is that we should go forward as is:

• We have no evidence that the back-end fails to set those values.
• Without them, we have no way to construct a Dataset / Table which could be used to make API requests (without a name), which means the KeyError is the earliest possible detection for the useless resource.

[dhermes]

I agree that we can be mostly secure in the belief that the backend will send good data and that without those keys, we can't do anything anyhow.

I just wanted to discuss the possibility that we would provide a more specific error message than KeyError: 'datasetReference' or KeyError: 'datasetId'.

[tseaver]

FWIW, I'd generally prefer not to re-wrap exceptions (losing the original traceback really damages debuggability, for instance).

[dhermes]

If we raise one line after the KeyError would have occurred, how does that damage debuggability? If the method fails and the user sees an error in a method they've never heard of, isn't that also pretty low quality debuggability?

[tseaver]

Hiding the key error is the harm I'm talking about: the user won't be better able to debug some other error more easily than "that key isn't in the resource as expected".

[dhermes]

Maybe we are thinking of different things.

This is what I have in mind:

if ('datasetReference' not in resource or
    'datasetId' not in resource['datasetReference']):
  raise KeyError('The resource returned from the server did not contain '
                 'the the necessary information to create a Dataset '
                 'object. The resource needs to contain a dictionary value '
                 'at the datasetReference key and within that dictionary '
                 'needs the datasetId.')
name = resource['datasetReference']['datasetId']

What harm does this cause? I'm just suggesting we provide more information than what a KeyError would on its own.

[tseaver]

When a Python programmer sees a traceback for a KeyError where the bottommost line is:

name = resource['datasetReference']['datasetId']

doesn't she already know the same information you typed into that waaay-long error message? If debugging it, she will still dump out the contents of resource and try to figure out why those keys are missing.

[dhermes]

Yes that's the root of my original question.

Do we want that Python programmer to just see that KeyError and try to figure out what resource was and where it came from, or do we want to give them more information which will explain why the error occurred. I was leaning towards the latter since it is not a method users would ever invoke, hence their knowledge of the inputs and failure modes would be minimal.

[tseaver]

@dhermes 34a747c adds a check such as you suggested.

[dhermes]

LGTM