Skip to content

[GOBBLIN-1674] : Add an option in gobblin.sh to specify whether to run the Gobblin job in background or not#3534

Open
bharos wants to merge 1 commit intoapache:masterfrom
bharos:GOBBLIN-1674
Open

[GOBBLIN-1674] : Add an option in gobblin.sh to specify whether to run the Gobblin job in background or not#3534
bharos wants to merge 1 commit intoapache:masterfrom
bharos:GOBBLIN-1674

Conversation

@bharos
Copy link
Copy Markdown
Contributor

@bharos bharos commented Aug 5, 2022

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots (if applicable):
    Currently, the command uses 'nohup' and '&' which always makes it run in background.
    This makes it inconvenient in use-cases that need to do some action after the run is
    completed (like curling for some logs). Hence, providing an option to choose whether
    to run in background or not.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    Tested in local environment with the new flag, unit tests Not applicable.

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

…n the Gobblin job in background or not

Currently, the command uses 'nohup' and '&' which always makes it run in background.
This makes it inconvenient in use-cases that need to do some action after the run is
completed (like curling for some logs). Hence, providing an option to choose whether
to run in background or not.
@abti
Copy link
Copy Markdown
Member

abti commented Aug 5, 2022

+1 It looks good to me.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 8, 2022

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.68%. Comparing base (9ce1e65) to head (a1b3d3f).
⚠️ Report is 583 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3534      +/-   ##
============================================
- Coverage     46.69%   46.68%   -0.01%     
- Complexity    10432    10436       +4     
============================================
  Files          2083     2083              
  Lines         81516    81516              
  Branches       9100     9100              
============================================
- Hits          38061    38056       -5     
- Misses        39944    39946       +2     
- Partials       3511     3514       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Will-Lo
Copy link
Copy Markdown
Contributor

Will-Lo commented Aug 8, 2022

@bharos Could you rebase from master? It conflicts with the previous changes that you've made

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.

4 participants