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.

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

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

Merged
merged 3 commits into from
Mar 26, 2019

Conversation

AVaksman
Copy link
Contributor

Fixes #7531

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

* 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
_INSTANCE_CREATE_WARNING.format(
"location_id", "serve_nodes", "default_storage_type"
),
PendingDeprecationWarning,
Copy link
Contributor

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
Contributor Author

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
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
Contributor Author

@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
Contributor

tseaver commented Mar 26, 2019

Unrelated test failures:

@tseaver tseaver merged commit ce71f8c into googleapis:master Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

5 participants