Skip to content

[ZEPPELIN-6509] Add error handling and safety guards to genthrift.sh - #5297

Merged
tbonelee merged 1 commit into
apache:masterfrom
celinayk:ZEPPELIN-6509
Jul 15, 2026
Merged

[ZEPPELIN-6509] Add error handling and safety guards to genthrift.sh#5297
tbonelee merged 1 commit into
apache:masterfrom
celinayk:ZEPPELIN-6509

Conversation

@celinayk

Copy link
Copy Markdown
Contributor

What is this PR for?

genthrift.sh (the Thrift code generation script) had no error handling. If the thrift compiler was missing or failed, or if the script was run from the wrong directory,
it could silently overwrite valid generated Java sources with empty/partial output. This PR adds basic safety guards so the script fails fast instead of corrupting the
source tree

What type of PR is it?

Bug Fix

Todos

  • Add set -euo pipefail so any failing command aborts the script
  • Add cd "$(dirname "${BASH_SOURCE[0]}")" so the script always runs relative to its own location, regardless of the caller's working directory
  • Check that java_license_header.txt exists before proceeding, and exit with a clear error message if it's missing

What is the Jira issue?

ZEPPELIN-6509

How should this be tested?

  • This is a developer-only shell script (not covered by the build/test suite), so testing is manual:
    1. bash -n zeppelin-interpreter/src/main/thrift/genthrift.sh — verify syntax
    2. Run the script from the correct directory with thrift installed — confirm it still regenerates sources as before
    3. Temporarily rename/remove java_license_header.txt and run the script — confirm it exits with an error instead of silently producing malformed output
    4. Run the script from a different working directory (e.g. repo root) — confirm it still operates on the correct paths instead of deleting an unintended directory

Screenshots (if appropriate)

N/A (shell script change, no UI)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@tboneleetbonelee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tbonelee
tbonelee merged commit be438e1 into apache:masterJul 15, 2026
16 of 19 checks passed
@tbonelee

Copy link
Copy Markdown
Contributor

Merged to master

Sign up for freeto 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

@celinayk@tbonelee