Skip to content

[SPARK-57221][INFRA] Make merge_spark_pr.py fail fast on missing JIRA setup and stale script versions#56282

Open
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:merge-script-require-jira
Open

[SPARK-57221][INFRA] Make merge_spark_pr.py fail fast on missing JIRA setup and stale script versions#56282
cloud-fan wants to merge 1 commit into
apache:masterfrom
cloud-fan:merge-script-require-jira

Conversation

@cloud-fan
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Harden dev/merge_spark_pr.py so it refuses to run when a prerequisite for a correct merge is missing, instead of silently proceeding:

  • initialize_jira() now fails fast when the jira library is not installed or when no JIRA credentials are configured, rather than prompting Continue without jira? and merging without resolving the ticket. It runs before any git work, so nothing is merged when it aborts.
  • Add check_script_up_to_date(), run first in main(), which asks GitHub for the latest commit on master touching this script and fails if that commit is not in the local history (i.e. the committer is missing an upstream update). It compares commit reachability rather than downloading the file, so being ahead of master is fine and does not trip the check. Set SKIP_VERSION_CHECK=1 to bypass when iterating on the script itself.
  • Drop the now-unreachable asf_jira is None guards and the misleading Exiting without trying to close the associated JIRA branch, since JIRA access is now mandatory.

Why are the changes needed?

When the jira library is not installed (or credentials are unset), the merge script would prompt to continue without JIRA and then merge the PR while leaving the associated ticket Open. This is easy to miss and tedious to reconcile after the fact. A stale local copy of the merge script can likewise reintroduce already-fixed behavior. Failing loudly on both makes a correct merge the only path.

Does this PR introduce any user-facing change?

No. This only affects committers running the merge tooling.

How was this patch tested?

  • python3 -m py_compile dev/merge_spark_pr.py and python3 -m doctest dev/merge_spark_pr.py.
  • Exercised check_script_up_to_date() for three paths: up-to-date/ahead (passes), SKIP_VERSION_CHECK=1 (bypasses), and a missing-upstream-commit SHA (loud fail() + non-zero exit).

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

This pull request and its description were written by Isaac.

… setup and stale script versions

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

Harden `dev/merge_spark_pr.py` so it refuses to run when a prerequisite for a
correct merge is missing, instead of silently proceeding:

- `initialize_jira()` now fails fast when the `jira` library is not installed or
  when no JIRA credentials are configured, rather than prompting
  "Continue without jira?" and merging without resolving the ticket. This runs
  before any git work, so nothing is merged when it aborts.
- Add `check_script_up_to_date()`, run first in `main()`, which asks GitHub for the
  latest commit on master touching this script and fails if that commit is not in
  the local history (i.e. the committer is missing an upstream update). It compares
  commit reachability rather than downloading the file, so being ahead of master is
  fine. Set `SKIP_VERSION_CHECK=1` to bypass when iterating on the script itself.
- Drop the now-unreachable `asf_jira is None` guards and the misleading
  "Exiting without trying to close the associated JIRA" branch.

### Why are the changes needed?

When the `jira` library is not installed (or credentials are unset), the merge
script would prompt to continue without JIRA and then merge the PR while leaving
the associated ticket Open. This is easy to miss and tedious to reconcile later.
A stale local copy of the merge script can likewise reintroduce already-fixed
behavior. Failing loudly on both makes a correct merge the only path.

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

No. This only affects committers running the merge tooling.

### How was this patch tested?

- `python3 -m py_compile dev/merge_spark_pr.py` and `python3 -m doctest dev/merge_spark_pr.py`.
- Exercised `check_script_up_to_date()` for the up-to-date/ahead (pass),
  `SKIP_VERSION_CHECK` (bypass), and missing-upstream-commit (loud fail) paths.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

Co-authored-by: Isaac
@cloud-fan
Copy link
Copy Markdown
Contributor Author

cc @dongjoon-hyun @yaooqinn

@cloud-fan
Copy link
Copy Markdown
Contributor Author

I'll find a PR to merge, to test this change

Copy link
Copy Markdown
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, this change sounds reasonable to me.

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