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.

Sign up for GitHub

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

Jump to bottom

Jobs: load from storage #1056

Merged
merged 16 commits into from
Aug 17, 2015
Merged

Jobs: load from storage #1056

merged 16 commits into from
Aug 17, 2015

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 12, 2015

Add support for jobs loading table data from CloudStorage files.

@tseaver tseaver added the api: bigquery Issues related to the BigQuery API. label Aug 12, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 12, 2015
@dhermes
Copy link
Contributor

dhermes commented Aug 12, 2015

Does this have a diffbase?

@tseaver
Copy link
Contributor Author

tseaver commented Aug 12, 2015

Does this have a diffbase?

Nope -- I rebased it before pushing. Too big?

@tseaver tseaver mentioned this pull request Aug 12, 2015
Merged
@dhermes
Copy link
Contributor

dhermes commented Aug 12, 2015

I just wasn't sure since there were 8 commits. Also, you're failing pep8 (that one runs fast, pylint is the slow one).

@dhermes
Copy link
Contributor

dhermes commented Aug 12, 2015

Also ISTM a rebase won't feel very good due to the merging of #1051.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 12, 2015

I will fix it up (both the pep8 and the date stuff) and re-push.

@dhermes
Copy link
Contributor

dhermes commented Aug 12, 2015

Thanks. Ping me when you push.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 12, 2015

Ugh! The pep8 fix was trivial, but working around the pylint 'too-many-branches' failures made me grind my teeth (4c14890)

@dhermes
Copy link
Contributor

dhermes commented Aug 12, 2015

This thing is all ready to review?

@tseaver tseaver added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 13, 2015
@tseaver
Copy link
Contributor Author

tseaver commented Aug 13, 2015

Hold off a bit -- I'm adding a system test for the "load from cloud storage" case, and it is failing.

@tseaver tseaver removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 13, 2015
@tseaver
Copy link
Contributor Author

tseaver commented Aug 13, 2015

@dhermes OK, got the system test added and passing.

dhermes
dhermes reviewed Aug 13, 2015
View reviewed changes
gcloud/bigquery/client.py
@@ -96,3 +97,20 @@ def dataset(self, name):
:returns: a new ``Dataset`` instance
"""
return Dataset(name, client=self)

def load_from_storage(self, name, destination, *source_uris):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Aug 17, 2015
@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2015

👍 to the enums. Why not just take the jump and use enum34?

Also it looks like you had a bad merge / rebase, you've got two commits by me jammed in the middle:

screen_shot_026

@tseaver
Match order of CUT methods.
062d5bb
@tseaver
Add 'bigquery.job.LoadFromStorageJob.
14fbb6c 
Models job request to load a BQ table from file(s) in CloudStorage.
@tseaver
Add 'Client.load_from_storage' factory.
7b31499 
Builds an instance of 'job.LoadFromStorageJob'.
@tseaver
Factor out schema resource helpers for re-use.
0c3232c 
- 'table.Table._parse_schema_resource' -> 'table._parse_schema_resource'
- 'table.Table._build_schema_resource' -> 'table._build_schema_resource'
@tseaver
Surface 'Table.project'/'Table.dataset_name' properties FBO 'Job'.
54607e8
@tseaver
Add 'LoadFromStorageJob.begin' API method.
cd1fdba
@tseaver
Add 'LoadFromStorageJob.exists' API method.
66dd746
@tseaver
Add 'LoadFromStorageJob.reload' API method.
4da9bb5
@tseaver
Accomodate datetime centralization from #1051.
15d8663
@tseaver
Include 'sourceUris', 'destination' in load configuration.
e37ab0b 
Found while attempting to add a system test for the 'load from storage' case.
@tseaver
Add system test for 'load from storage' case.
f88dcc3
@tseaver
Add docstring.
31026aa 
Addresses:
https://github.com/GoogleCloudPlatform/gcloud-python/pull/1056/files#r37027413
@tseaver
Use a longer var name for temporary file.
31c4b1b 
Addresses:
https://github.com/GoogleCloudPlatform/gcloud-python/pull/1056/files#r37028059
@tseaver
Implement 'cancel' API request.
6549219 
Addresses:
#1056 (comment)
@tseaver
Expose quasi-enum types for string values w/ choices.
9260ae9 
Users who want to avoid tripping over string literal mismatches can use
class constants instead.

Addresses:
https://github.com/GoogleCloudPlatform/gcloud-python/pull/1056/files#r37029533
@tseaver
Make load job name / API more explicit:
3e10e52 
- 'Client.load_from_storage' -> 'Client.load_table_from_storage'.
- 'job.LoadFromStorageJob' -> 'job.LoadTableFromStorageJob'.

Addresses:

https://github.com/GoogleCloudPlatform/gcloud-python/pull/1056/files#r37120652
@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Aug 17, 2015
@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2015

Why not just take the jump and use enum34?

I didn't want to eat the extra dependency. We have #1031 to track the idea, though.

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2015

Why not? It's lightweight and hopefully most users already have it

>>> import enum
>>> 
>>> class RedBlue(enum.Enum):
...     RED = 'RED'
...     BLUE = 'BLUE'
... 
>>> 
>>> to_validate = 'RED'
>>> rb = RedBlue(to_validate)
>>> print(rb)
RedBlue.RED
>>> print(rb.value)
RED
>>> 
>>> to_validate = 'GREEN'
>>> try:
...     RedBlue(to_validate)
... except ValueError as exc:
...     print(repr(exc))
...     print('%s was not validated' % (to_validate,))
... 
ValueError('GREEN is not a valid RedBlue',)
GREEN was not validated

@tseaver
Copy link
Contributor Author

tseaver commented Aug 17, 2015

Most users are on Python2, and likely won't have it unless some other library has pushed them to add it. Again, we can clean up here (and in the other APIs) when we address #1031.

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2015

LGTM. I really hope the follow-up commits can spread out some of the pain here.

I was saying that most Python 2 only users would have lots of libraries installed that straddled Py2 and Py3, and hence would have a good chance that one of them used enum34. (I could've been clearer there.)

@dhermes
Copy link
Contributor

dhermes commented Aug 17, 2015

@tseaver Please ping me on anything that will get rebased after this goes in.

tseaver added a commit that referenced this pull request Aug 17, 2015
@tseaver
Merge pull request #1056 from tseaver/bigquery-jobs_load_from_storage
cf93465 
Jobs: load from storage
@tseaver tseaver merged commit cf93465 into googleapis:master Aug 17, 2015
@tseaver tseaver deleted the bigquery-jobs_load_from_storage branch August 17, 2015 22:34
@tseaver tseaver mentioned this pull request Aug 18, 2015
Merged
@dhermes dhermes mentioned this pull request Sep 30, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tseaver @dhermes @googlebot