Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4242: Updated Deploy and Pull scripts for TravisCI to report it as failed on issue #4276

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abhi-bhatra
Copy link

Expected Behaviour

Deploy and Pull Scripts should send exit code 1 when failure occurs

Actual Behaviour

From the example, deploy.sh script is sending exit code 0, which means success and Travis CI, in return make build successful.
image

Fix:

  1. Deploy.sh: Create a function to check for AWS_ACCOUNT_ID, and this function will only be called for the cases which uses this variable, for others cases, it will not check for this value.

  2. Pull.sh: if env is production/staging, and error occurs in build_and_push function, it exits with error code 1.

@codecov-commenter
Copy link

Codecov Report

Merging #4276 (7532340) into master (96968d6) will decrease coverage by 3.63%.
Report is 1101 commits behind head on master.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4276      +/-   ##
==========================================
- Coverage   72.93%   69.30%   -3.63%     
==========================================
  Files          83       20      -63     
  Lines        5368     3574    -1794     
==========================================
- Hits         3915     2477    -1438     
+ Misses       1453     1097     -356     

see 64 files with indirect coverage changes

see 64 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f705b19...7532340. Read the comment docs.

@Suryansh5545
Copy link
Member

@abhi-bhatra I believe you have taken the wrong example from the PR i referenced. I was referring to this build https://app.travis-ci.com/github/Cloud-CV/EvalAI/builds/267908712#L7297 Which is on staging and when the script fails it still reports success. The exit 0 on master branch is completely fine and expected

@abhi-bhatra
Copy link
Author

Hi @Suryansh5545 you are referring to this build failure, where django installation failed, right ?

image

@Suryansh5545
Copy link
Member

Hi @Suryansh5545 you are referring to this build failure, where django installation failed, right ?

image

@abhi-bhatra Yes, we want to make sure that if the scripts exits due to any kind of error then the exit is set to 1 instead of 0. To indicate failure of deployment

@abhi-bhatra
Copy link
Author

abhi-bhatra commented Mar 12, 2024

Hi @Suryansh5545 as you said that the exit 0 was just fine in deploy and push scripts and working as expected. Then, I believe there is an issue with Travis yml manifest.
There is a flag set -e is set in both the scripts, which send a non-zero exit code immediately, the script fails on any step. I have set a travis_terminate 1; which exits the pipeline if any of the script is failed.

Kindly check the new changes pushed !

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.

None yet

4 participants