Skip to content

fix(compute): replace missing REQUIRED for parent_id#711

Merged
noahdietz merged 1 commit intomasterfrom
firewall-policy-parent-id
Apr 12, 2022
Merged

fix(compute): replace missing REQUIRED for parent_id#711
noahdietz merged 1 commit intomasterfrom
firewall-policy-parent-id

Conversation

@noahdietz
Copy link
Copy Markdown
Contributor

@noahdietz noahdietz commented Apr 8, 2022

The REQUIRED field_behavior annotation for InsertFirewallPolicyRequest.parent_id (added in 02df998) was accidentally removed in 249e9a1. This caused a backwards incompatible change for the PHP GAPIC.

cc: @vchudnov-g

@noahdietz noahdietz requested a review from vam-google April 8, 2022 18:13
@mistaken-pull-closer
Copy link
Copy Markdown

mistaken-pull-closer Bot commented Apr 8, 2022

Thanks for your contribution! Unfortunately, we don't use GitHub pull
requests to manage code contributions to this repository. Instead, please
see CONTRIBUTING.md which provides full
instructions on how to get involved.

@product-auto-label product-auto-label Bot added the api: compute Issues related to the Compute Engine API. label Apr 8, 2022
@noahdietz noahdietz reopened this Apr 8, 2022
@vchudnov-g
Copy link
Copy Markdown
Contributor

vchudnov-g commented Apr 11, 2022

Where did the "accident" happen? Was it a bug in the pipeline code, or something upstream in our inputs?

@vchudnov-g
Copy link
Copy Markdown
Contributor

vchudnov-g commented Apr 12, 2022 •
edited
Loading

For context: we had added REQUIRED in ana earlier commit as a manual fix, but that got overwritten when we auto-generated the synthetic protos again. That is why we have to re-apply the manual fix here. We believe we have machinery in place so that this fix is propagated in future synthetic proto generations.

@noahdietz noahdietz merged commit 4bb6fd6 into master Apr 12, 2022
@noahdietz noahdietz deleted the firewall-policy-parent-id branch April 12, 2022 16:51
@vchudnov-g
Copy link
Copy Markdown
Contributor

vchudnov-g commented Apr 12, 2022

Correction: the machinery for propagating the annotation fix in the future is not yet in place, but @vam-google will work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vchudnov-g vchudnov-g vchudnov-g approved these changes

@vam-google vam-google Awaiting requested review from vam-google

Assignees

No one assigned

Labels

api: compute Issues related to the Compute Engine API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@noahdietz @vchudnov-g