Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 14, 2024

What do these changes do?

The issue was caused by our CI pipeline not using a pinned version of mypy; instead, it was always fetching the latest version. This happened because requirements/ci.txt did not include requirements/tools.txt, which specifies the version for mypy.

As a result, mypy was updated between the CI passing for this PR and its merge into the master branch (commit da15add).

The new version of mypy introduced additional type checks that uncovered issues in some parts of the code. Specifically, we had to make further changes to the section handling invoice attachments in emails to resolve these newly identified issues.

Highlights

  • ♻️ payments service:
    • Proper attachment and better error handling to invoice attachment.
    • Adapts tests
    • @matusdrobuliak66 please pay special attention to the changes i did of your code in
      - services/payments/src/simcore_service_payments/services/notifier_email.py
      - services/payments/tests/unit/test_services_notifier_email.py
  • ♻️ ensures create_troubleshotting_log_message does not raise
  • 🔨 uses uv run to run script

Related issue/s

How to test

Dev-ops checklist

NOne

@pcrespov pcrespov self-assigned this Oct 14, 2024
@codecov
Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 90.69767% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.1%. Comparing base (cafbf96) to head (7b3df84).
Report is 628 commits behind head on master.

Files with missing lines Patch % Lines
...imcore_service_payments/services/notifier_email.py 91.6% 3 Missing ⚠️
...models_library/utils/_original_fastapi_encoders.py 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6527      +/-   ##
=========================================
+ Coverage    84.5%   88.1%    +3.5%     
=========================================
  Files          10    1548    +1538     
  Lines         214   63351   +63137     
  Branches       25    2059    +2034     
=========================================
+ Hits          181   55845   +55664     
- Misses         23    7188    +7165     
- Partials       10     318     +308     
Flag Coverage Δ
integrationtests 64.7% <ø> (?)
unittests 86.1% <90.6%> (+1.5%) ⬆️

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

Files with missing lines Coverage Δ
...es/service-library/src/servicelib/logging_utils.py 77.8% <100.0%> (ø)
packages/service-library/src/servicelib/utils.py 91.6% <100.0%> (ø)
...es/service-library/src/servicelib/utils_secrets.py 92.3% <100.0%> (ø)
...models_library/utils/_original_fastapi_encoders.py 51.2% <0.0%> (ø)
...imcore_service_payments/services/notifier_email.py 95.1% <91.6%> (ø)

... and 1493 files with indirect coverage changes

@pcrespov pcrespov added t:maintenance Some planned maintenance work a:models-library labels Oct 14, 2024
@pcrespov pcrespov marked this pull request as ready for review October 14, 2024 14:36
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@pcrespov pcrespov requested a review from GitHK as a code owner October 14, 2024 14:42
@pcrespov pcrespov changed the title 🔨 Unexpected mypy failure in models-library ♻️🔨 Unexpected mypy upgrade surfaced some failures in configuration and code Oct 14, 2024
@pcrespov pcrespov added this to the MartinKippenberger milestone Oct 14, 2024
@pcrespov pcrespov changed the title ♻️🔨 Unexpected mypy upgrade surfaced some failures in configuration and code ♻️🔨 Unexpected mypy upgrade revealed configuration and code failures Oct 14, 2024
@pcrespov pcrespov enabled auto-merge (squash) October 14, 2024 17:37
@sonarqubecloud
Copy link

Copy link
Contributor

@jsaq007 jsaq007 left a comment

Choose a reason for hiding this comment

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

👍

@pcrespov pcrespov merged commit b25d613 into ITISFoundation:master Oct 15, 2024
57 checks passed
@pcrespov pcrespov deleted the mai/fix-typecheck-master branch October 15, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:models-library t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants