Skip to content

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

Open
celinayk wants to merge 1 commit into
apache:masterfrom
celinayk:ZEPPELIN-6509
Open

[ZEPPELIN-6509] Add error handling and safety guards to genthrift.sh#5297
celinayk wants to merge 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

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.

1 participant