FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deprecate(bigquery): deprecate `client.dataset()` in favor of `DatasetReference` by tswast · Pull Request #7753 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

deprecate(bigquery): deprecate client.dataset() in favor of DatasetReference - #7753

Merged
plamut merged 4 commits into
googleapis:masterfrom
tswast:bq-dot-dataset-deprecation
Jan 27, 2020
ConversationCommits (4)ChecksFiles changed
Merged

deprecate(bigquery): deprecate client.dataset() in favor of DatasetReference#7753
plamut merged 4 commits into
googleapis:masterfrom
tswast:bq-dot-dataset-deprecation

Conversation

tswast commented Apr 19, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Now that all client methods that take a DatasetReference or
TableReference also take a string, the client.dataset() method is
unnecessary and confusing.

Closes #8989.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 19, 2019

tswast commented Apr 19, 2019

Copy link
Copy Markdown
Contributor Author

@crwilcox I was inspired by our conversation today.

Note: I've created this as a draft pull request because after I made this commit, I realized we still call .dataset() 96 times in our samples at docs/snippets.py, and that's after @lbristol88 already moved a bunch over to the new "just use a string instead of a reference" way of doing things! I would feel better about adding the deprecation notice after we update the rest of our samples.

yoshi-automation added the 🚨 This issue needs some love. label Apr 26, 2019

busunkim96 commented Jun 10, 2019

Copy link
Copy Markdown
Contributor

@tswast Gentle ping to see if there are any updates on this PR.

tswast commented Jun 10, 2019

Copy link
Copy Markdown
Contributor Author

@busunkim96 I've worked with @sduskis to allocate some contractor resources for the necessary code sample updates before we unleash this PR.

Is it an issue if we keep this open for now?

busunkim96 commented Jun 10, 2019

Copy link
Copy Markdown
Contributor

@tswast Nope, I just wanted to check. 😸 Thanks!

yoshi-automation removed the 🚨 This issue needs some love. label Jul 3, 2019
tswast mentioned this pull request Aug 7, 2019
Closed
tswast closed this Dec 27, 2019
tswast deleted the bq-dot-dataset-deprecation branch December 27, 2019 21:21
tswast restored the bq-dot-dataset-deprecation branch December 29, 2019 00:04
tswast reopened this Dec 29, 2019
tswast force-pushed the bq-dot-dataset-deprecation branch from 074af14 to 5a1bab8 Compare January 23, 2020 20:18
deprecate(bigquery): deprecate client.dataset() in favor of Dataset…
cec97ff 
…Reference

Now that all client methods that take a `DatasetReference` or
`TableReference` also take a string, the `client.dataset()` method is
unnecessary and confusing.
tswast force-pushed the bq-dot-dataset-deprecation branch from 5a1bab8 to cec97ff Compare January 23, 2020 20:19
tswast changed the title Document client.dataset() deprecation. deprecate(bigquery): deprecate client.dataset() in favor of DatasetReference Jan 23, 2020
tswast marked this pull request as ready for review January 23, 2020 20:20
tswast requested review from a team, plamut and shollyman January 23, 2020 20:20
plamut reviewed Jan 24, 2020
View reviewed changes

plamut 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are still a lot of client.dataset() calls in snippets.py, but that file is deprecated, IIRC.

The rest looks good, aside from a remark on a possible grammar thingy.

Comment thread bigquery/google/cloud/bigquery/client.py Outdated
Comment on lines +360 to +363
This method is deprecated. Construct a
:class:`~google.cloud.bigquery.dataset.DatasetReference` using its
constructor or use a string where previously a reference object was
used.

plamut Jan 24, 2020

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would it make sense to use the standard Sphinx directive for deprecations?
The text itself is already clear, though.

tswast Jan 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Doesn't appear to change formatting all that much:

Pushed the change in case it makes things a little more machine-readable or something, though.

Comment thread bigquery/google/cloud/bigquery/client.py Outdated

plamut commented Jan 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

The test_client_load_partitioned_table snippet test failure could be unrelated, happens on another PR, too.

Might still be worth investigating, though.

Edit: Also happens on master, thus indeed unrelated to this PR.

Update: #10195 fixes it.

Update bigquery/google/cloud/bigquery/client.py
50ac8fe 
Co-Authored-By: Peter Lamut <plamut@users.noreply.github.com>

tswast commented Jan 24, 2020

Copy link
Copy Markdown
Contributor Author

There are still a lot of client.dataset() calls in snippets.py, but that file is deprecated, IIRC.

That's right. We're moving those samples to the samples/ directory and updating them to avoid the use of client.dataset(). I'm hoping that with #10175 and #10176 we'll have enough snippets updated.

plamut approved these changes Jan 27, 2020
View reviewed changes

plamut 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good now. 👍

plamut merged commit 7e36fda into googleapis:master Jan 27, 2020
tswast deleted the bq-dot-dataset-deprecation branch January 27, 2020 16:22
yoshi-automation mentioned this pull request Jan 29, 2020
Merged
This was referenced Jan 29, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Closed
yoshi-automation mentioned this pull request Feb 7, 2020
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

shollyman Awaiting requested review from shollyman
1 more reviewer
plamut plamut approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

BigQuery: deprecate client.dataset()

5 participants

Back | FazBrowse Home | New Git URL