Skip to content

fix: do not allow empty key parts for key constructor in namespaced model#401

Merged
cguardia merged 7 commits intogoogleapis:masterfrom
cguardia:model_constructor_key_parts
Apr 29, 2020
Merged

fix: do not allow empty key parts for key constructor in namespaced model#401
cguardia merged 7 commits intogoogleapis:masterfrom
cguardia:model_constructor_key_parts

Conversation

@cguardia
Copy link
Contributor

refs #384

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 26, 2020
@cguardia cguardia added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 27, 2020
@cguardia cguardia requested a review from chrisrossi April 27, 2020 07:51
Copy link
Contributor

@chrisrossi chrisrossi left a comment

Choose a reason for hiding this comment

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

I think given that this isn't the first issue with serialization, it might make sense to have some regression tests explicitly for pickling/unpickling, so we can make sure that continues to work. Perhaps we could borrow the customer's tests from #384

@cguardia
Copy link
Contributor Author

@chrisrossi right you are. Added tests for serialization.

Copy link
Contributor

@chrisrossi chrisrossi left a comment

Choose a reason for hiding this comment

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

👍


@pytest.mark.usefixtures("client_context")
def test_serialization(dispose_of):
"""Regression test for #843
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in issue #

@cguardia cguardia merged commit f3528b3 into googleapis:master Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants