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: exit 1 when logging error messages #606

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Conversation

klmcadams
Copy link
Contributor

@klmcadams klmcadams commented Oct 10, 2024

Add check to see if bot user or email inputs are empty. If so, log an error message & exit 1. Exiting 1 when logging an error message was applied to the _logging action.

Exit 1 on error
Bot secrets exist, so no error

Closes #603

@klmcadams klmcadams requested a review from a team as a code owner October 10, 2024 16:45
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the fix Pull requests related to resolving problems or errors label Oct 10, 2024
Copy link
Contributor

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

I think doing exit 1 when we are loggin an error makes sense... however have you checked that you are not logging anywhere else with ERROR level?
Alternatively, you could do that exit optionally using another argument (force_exit maybe?)

Other than that, it looks good to me.

@klmcadams
Copy link
Contributor Author

It looks like the other locations logging with ERROR are _doc-build-windows and _doc-build-linux when it's unable to determine the PDF filename using the conf.py file. If being unable to determine the PDF filename isn't a dealbreaker & the action can still pass without it, I think the logging level for those cases should change to WARNING instead

Copy link
Contributor

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

LGTM For the doc-build actions I think it's great to have this early exit because if the error logs are triggered, it means that the job using the action will fail fore sure.

@SMoraisAnsys
Copy link
Contributor

It looks like the other locations logging with ERROR are _doc-build-windows and _doc-build-linux when it's unable to determine the PDF filename using the conf.py file. If being unable to determine the PDF filename isn't a dealbreaker & the action can still pass without it, I think the logging level for those cases should change to WARNING instead

It would break further steps in the action if one triggers any of the error log.

Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

LGTM!

@klmcadams klmcadams merged commit 6c9c192 into main Oct 11, 2024
14 checks passed
@klmcadams klmcadams deleted the fix/exit-error-logging branch October 11, 2024 12:17
@RobPasMue
Copy link
Member

Releasing this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests related to resolving problems or errors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Username and email inputs not throwing error
4 participants