Skip to content

Bigtable: cleanup unused variable and add deprecation warnings for to be removed features#7532

Merged
tseaver merged 3 commits intogoogleapis:masterfrom
AVaksman:cleanup_and_deprication_warnings
Mar 26, 2019
Merged

Bigtable: cleanup unused variable and add deprecation warnings for to be removed features#7532
tseaver merged 3 commits intogoogleapis:masterfrom
AVaksman:cleanup_and_deprication_warnings

Conversation

@AVaksman
Copy link
Copy Markdown
Contributor

@AVaksman AVaksman commented Mar 18, 2019

Fixes #7531

  • some cleanup
  • add deprecation warnings for backwards compatible features that will go away

@AVaksman
* some cleanup
d0e8741 
* add deprecation warnings for backwards compatible features that will go away
@AVaksman AVaksman requested a review from crwilcox as a code owner March 18, 2019 20:20
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 18, 2019
@AVaksman AVaksman changed the title Bigtable: cleanup and deprivation warnings Bigtable: cleanup and deprecation warnings Mar 18, 2019
@AVaksman AVaksman changed the title Bigtable: cleanup and deprecation warnings Bigtable: cleanup unused variable and add deprecation warnings for to be removed features Mar 18, 2019
@AVaksman AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 19, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 19, 2019
@tseaver tseaver added the api: bigtable Issues related to the Bigtable API. label Mar 19, 2019
tseaver
tseaver suggested changes Mar 19, 2019
View reviewed changes
bigtable/google/cloud/bigtable/instance.py Outdated
_INSTANCE_CREATE_WARNING.format(
"location_id", "serve_nodes", "default_storage_type"
),
PendingDeprecationWarning,
Copy link
Copy Markdown
Contributor

@tseaver tseaver Mar 19, 2019

Choose a reason for hiding this comment

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

I think we can make this just a regular DeprecationWarning. Thanks for adding stacklevel=2!

Copy link
Copy Markdown
Contributor Author

@AVaksman AVaksman Mar 20, 2019

Choose a reason for hiding this comment

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

Done, thanks

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Mar 19, 2019

@AVaksman We need to update the unit tests to catch / assert the new warnings being raised, e.g.:

    def test_foo(self):
        import warnings

        with warnings.catch_warnings(record=True) as warned:
            foo()

        self.assertEqual(len(warned), 1)
        self.assertTrue(issubclass(warned[0].category, DeprecationWarning)

@AVaksman
Copy link
Copy Markdown
Contributor Author

AVaksman commented Mar 20, 2019

@AVaksman We need to update the unit tests to catch / assert the new warnings being raised, e.g.:

    def test_foo(self):
        import warnings

        with warnings.catch_warnings(record=True) as warned:
            foo()

        self.assertEqual(len(warned), 1)
        self.assertTrue(issubclass(warned[0].category, DeprecationWarning)

Done, thanks

@AVaksman AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 20, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 20, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 25, 2019
@AVaksman AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 26, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 26, 2019
@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Mar 26, 2019

Unrelated test failures:

@tseaver tseaver merged commit ce71f8c into googleapis:master Mar 26, 2019
parthea pushed a commit that referenced this pull request Nov 22, 2025
@AVaksman @tseaver
Add deprecation warnings for to-be-removed features. (#7532)
15b92ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crwilcox crwilcox Awaiting requested review from crwilcox

1 more reviewer

@tseaver tseaver tseaver approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement. 🚨 This issue needs some love.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AVaksman @tseaver @googlebot @yoshi-automation @yoshi-kokoro