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

Flip --experimental_worker_allow_json_protocol #13745

Conversation

keith
Copy link
Member

@keith keith commented Jul 26, 2021

Based on the discussion here
#13607 we want to flip this and
leave it around temporarily, but there are also some refactorings coming
around how to do that. This flips the value now so it can be used
without the flag sooner in rolling releases.

Based on the discussion here
bazelbuild#13607 we want to flip this and
leave it around temporarily, but there are also some refactorings coming
around how to do that. This flips the value now so it can be used
without the flag sooner in rolling releases.
@larsrc-google
Copy link
Contributor

FYI: This change is correct, but I first have to make some internal adjustments. We have some cases where Blaze and workers are released out of sync, so we can't just flip this flag internally, because we can't tell the workers which one to use.

@aiuto
Copy link
Contributor

aiuto commented Dec 18, 2021

friendly ping. Can we move this along. It seems mergable.

@keith
Copy link
Member Author

keith commented Dec 20, 2021

Now that the ndjson change merged I assume the blocking changes are internal? Is that correct @larsrc-google ?

@bazel-io bazel-io closed this in 9e16a64 Jan 24, 2022
@keith keith deleted the ks/flip-experimental_worker_allow_json_protocol branch January 24, 2022 17:18
@keith
Copy link
Member Author

keith commented Jan 24, 2022

@Wyverald can we cherry pick this for the next 5.x release?

@Wyverald
Copy link
Member

We shouldn't flip incompatible flags in the middle of an LTS release. This has to wait until 6.x.

@keith
Copy link
Member Author

keith commented Jan 24, 2022

@Wyverald this isn't a breaking change though, since workers can only be one or the other, so I think this is purely additive?

@Wyverald
Copy link
Member

Ah, I see -- so flipping this flag only enables an extra feature that was already checked in? Then it indeed seems fine.

@keith
Copy link
Member Author

keith commented Jan 24, 2022

Yep, it was just a warning so that the API could be safely changed and users shouldn't be surprised because they had to pass the flag

@Wyverald Wyverald added this to the 5.1 release blockers milestone Jan 24, 2022
@aiuto
Copy link
Contributor

aiuto commented Jan 24, 2022 via email

@Wyverald
Copy link
Member

Wyverald commented Feb 3, 2022

@bazel-io fork 5.1

@Wyverald Wyverald removed this from the 5.1 release blockers milestone Feb 3, 2022
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Feb 8, 2022
Based on the discussion here
bazelbuild#13607 we want to flip this and
leave it around temporarily, but there are also some refactorings coming
around how to do that. This flips the value now so it can be used
without the flag sooner in rolling releases.

Closes bazelbuild#13745.

PiperOrigin-RevId: 423831532
(cherry picked from commit 9e16a64)
Wyverald pushed a commit that referenced this pull request Feb 9, 2022
Based on the discussion here
#13607 we want to flip this and
leave it around temporarily, but there are also some refactorings coming
around how to do that. This flips the value now so it can be used
without the flag sooner in rolling releases.

Closes #13745.

PiperOrigin-RevId: 423831532
(cherry picked from commit 9e16a64)

Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants