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

fix: Correct the field mask when adding labels to an instance that had none previously #114

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

dazuma
Copy link
Member

@dazuma dazuma commented Aug 15, 2024

Protobuf 4.x had a breaking change that affected how proto messages are converted to hashes. This affected the field mask computation when setting instance attributes—in particular, when using protobuf 4.x, it was no longer possible to set labels on an instance that had none previously because the field mask was omitting the labels field. This fix updates the field mask computation to be immune to the change.

@aandreassa
Copy link

I think this needs to be regenerated, similar to googleapis/google-cloud-ruby#26510

/workspace/google-cloud-spanner/benchmark/benchwrapper/spanner_pb.rb:20:in `': undefined method `build' for # (NoMethodError)

Google::Protobuf::DescriptorPool.generated_pool.build do`

@aandreassa
Copy link

LGTM. Just some other error on the build now. Something like:
/usr/grte/v5/lib64/libc.so.6: version GLIBC_ABI_DT_RELR' not found`

@dazuma
Copy link
Member Author

dazuma commented Aug 19, 2024

LGTM. Just some other error on the build now. Something like: /usr/grte/v5/lib64/libc.so.6: version GLIBC_ABI_DT_RELR' not found`

Yeah that's been there for a while on that job. We may need help from the spanner team, as I'm not sure what that job is.

@dazuma dazuma merged commit 22806e9 into googleapis:main Aug 19, 2024
14 of 15 checks passed
@dazuma dazuma deleted the pr/field-mask branch August 20, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants