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

Fixes for CodeQL #2449

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Fixes for CodeQL #2449

merged 1 commit into from
Sep 11, 2023

Conversation

judysng
Copy link
Contributor

@judysng judysng commented Sep 10, 2023

Description of changes

CodeQL findings:

  • In clusterstatusmgtd.py, adding a "return None" at the end to mitigate the implicit return. Case is not specified for what to return when the parameter "raise_on_error" is false when an error occurs, so returning an explicit None.
  • Also fixed small typo in clusterstatusmgtd.py
  • In cloudwatch_agent_config_util.py, added an explanatory comment for why there's a pass for a FileNotFoundError.

Going to Security > Code Scanning and typing "pr:2449" shows that the CodeQL alerts don't show up anymore!

References

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Sep 10, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.04% ⚠️

Comparison is base (3570025) 70.53% compared to head (3f789db) 70.49%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2449      +/-   ##
===========================================
- Coverage    70.53%   70.49%   -0.04%     
===========================================
  Files           13       13              
  Lines         1863     1864       +1     
===========================================
  Hits          1314     1314              
- Misses         549      550       +1     
Flag Coverage Δ
unittests 70.49% <66.66%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...fleet/files/clusterstatusmgtd/clusterstatusmgtd.py 74.70% <66.66%> (-0.30%) ⬇️

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

@judysng judysng marked this pull request as ready for review September 11, 2023 19:14
@judysng judysng requested review from a team as code owners September 11, 2023 19:14
@judysng judysng requested a review from himani2411 September 11, 2023 19:15
@himani2411
Copy link
Contributor

Could you add more descriptive commit messages.

@himani2411
Copy link
Contributor

Also, Can you have a look at which unit tests might be covering the tests as there are codecov checks which mention that the coverage is reduced.

…mment in cloudwatch_agent_config_util.py

Signed-off-by: Judy Ng <[email protected]>
@judysng judysng merged commit 71c252b into aws:develop Sep 11, 2023
25 of 27 checks passed
judysng added a commit to judysng/aws-parallelcluster-cookbook that referenced this pull request Sep 12, 2023
…mment in cloudwatch_agent_config_util.py (aws#2449)

Signed-off-by: Judy Ng <[email protected]>
hgreebe pushed a commit to hgreebe/aws-parallelcluster-cookbook that referenced this pull request Nov 13, 2023
…mment in cloudwatch_agent_config_util.py (aws#2449)

Signed-off-by: Judy Ng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants