Skip to content

[SPARK-41196][CONNECT][FOLLOW-UP] Fix out of sync generated files for Python#38718

Closed
amaliujia wants to merge 1 commit intoapache:masterfrom
amaliujia:fix_out_of_sync_proto
Closed

[SPARK-41196][CONNECT][FOLLOW-UP] Fix out of sync generated files for Python#38718
amaliujia wants to merge 1 commit intoapache:masterfrom
amaliujia:fix_out_of_sync_proto

Conversation

@amaliujia
Copy link
Contributor

@amaliujia amaliujia commented Nov 18, 2022

What changes were proposed in this pull request?

Fix out of sync generated files for Python.

This happens on a rare case for protobuf version change. #38693 downgraded protobuf versions.

There were something not generated before but with the protobuf version downgraded that was generated (and this is why there was no merge conflict). However the downgrading PR was based on old code before #38638 so the protobuf generates based on stale code which leads to stale generated files.

The way to better avoid this is when upon such change, it should lock the repo (partially on some directory to reduce impact), do the work, merge, and enforce pending PR to rebase. However this is not feasible (or too heavy) for concurrent development on Spark repo.

Another approach is to ask relevant PR to rebase, re-generate for python then merge. Though there could be still a gap here (a PR merged right right before merge button clicked) but that is minimized.

Why are the changes needed?

Fix out of sync generated files for Python.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

UT

@amaliujia
Copy link
Contributor Author

@zhengruifeng @HyukjinKwon

@HyukjinKwon
Copy link
Member

Merged to master.

@zhengruifeng
Copy link
Contributor

late lgtm

@zhengruifeng
Copy link
Contributor

it seems that the versions in build_and_test are not updated?

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
… Python

### What changes were proposed in this pull request?

Fix out of sync generated files for Python.

This happens on a rare case for protobuf version change. apache#38693 downgraded protobuf versions.

There were something not generated before but with the protobuf version downgraded that was generated (and this is why there was no merge conflict). However [the downgrading PR](apache#38693) was based on old code before apache#38638 so the protobuf generates based on stale code which leads to stale generated files.

The way to better avoid this is when upon such change, it should lock the repo (partially on some directory to reduce impact), do the work, merge, and enforce pending PR to rebase. However this is not feasible (or too heavy) for concurrent development on Spark repo.

### Why are the changes needed?

Fix out of sync generated files for Python.

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

UT

Closes apache#38718 from amaliujia/fix_out_of_sync_proto.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 15, 2022
… Python

### What changes were proposed in this pull request?

Fix out of sync generated files for Python.

This happens on a rare case for protobuf version change. apache#38693 downgraded protobuf versions.

There were something not generated before but with the protobuf version downgraded that was generated (and this is why there was no merge conflict). However [the downgrading PR](apache#38693) was based on old code before apache#38638 so the protobuf generates based on stale code which leads to stale generated files.

The way to better avoid this is when upon such change, it should lock the repo (partially on some directory to reduce impact), do the work, merge, and enforce pending PR to rebase. However this is not feasible (or too heavy) for concurrent development on Spark repo.

### Why are the changes needed?

Fix out of sync generated files for Python.

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

UT

Closes apache#38718 from amaliujia/fix_out_of_sync_proto.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
… Python

### What changes were proposed in this pull request?

Fix out of sync generated files for Python.

This happens on a rare case for protobuf version change. apache#38693 downgraded protobuf versions.

There were something not generated before but with the protobuf version downgraded that was generated (and this is why there was no merge conflict). However [the downgrading PR](apache#38693) was based on old code before apache#38638 so the protobuf generates based on stale code which leads to stale generated files.

The way to better avoid this is when upon such change, it should lock the repo (partially on some directory to reduce impact), do the work, merge, and enforce pending PR to rebase. However this is not feasible (or too heavy) for concurrent development on Spark repo.

### Why are the changes needed?

Fix out of sync generated files for Python.

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

UT

Closes apache#38718 from amaliujia/fix_out_of_sync_proto.

Authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants