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(api): Fix malformed PAPI error message #16065

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

SyntaxColoring
Copy link
Contributor

Overview

This fixes a malformed error message caught by the snapshot tests in #16062:

Nozzle layout configuration of style SINGLE is unsupported in API Versions lower than 2.20. is not yet available in the API version in use.

Test Plan and Hands on Testing

I'm relying on automated tests.

Changelog

  • Rework the underlying exceptions so that if you construct them with just a single unnamed string argument, it's used directly as the message, without any surprise concatenation. I think this is least surprising to the caller, given our codebase's existing exception conventions.

Also, for niceness:

  • typing.overloads, for clarity.
  • In the mode where we generate the exception message automatically from api_element+since_version+current_version, handle some additional cases of partially missing information.

Review requests

None.

Risk assessment

Low.

@SyntaxColoring SyntaxColoring requested a review from a team as a code owner August 20, 2024 15:03
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 11.11111% with 24 lines in your changes missing coverage. Please review.

Please upload report for BASE (chore_release-8.0.0@3d04c5c). Learn more about missing BASE report.

Files Patch % Lines
.../python/opentrons_shared_data/errors/exceptions.py 11.11% 24 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                  Coverage Diff                   @@
##             chore_release-8.0.0   #16065   +/-   ##
======================================================
  Coverage                       ?   73.27%           
======================================================
  Files                          ?       41           
  Lines                          ?     2945           
  Branches                       ?        0           
======================================================
  Hits                           ?     2158           
  Misses                         ?      787           
  Partials                       ?        0           
Flag Coverage Δ
shared-data 73.27% <11.11%> (?)

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

Files Coverage Δ
.../python/opentrons_shared_data/errors/exceptions.py 55.20% <11.11%> (ø)

@SyntaxColoring
Copy link
Contributor Author

SyntaxColoring commented Aug 20, 2024

Hmm. Not sure why the snapshot tests on this PR aren't showing a diff.

Verified manually that the fix works:

cd api
pipenv run python -m opentrons.cli analyze ../analyses-snapshot-testing/files/protocols/Flex_S_v2_16_P1000_96_TC_PartialTipPickupSingle.py --human-json-output -
// ...
{
"detail": "APIVersionError [line 16]: Error 4011 INCORRECT_API_VERSION (APIVersionError): Nozzle layout configuration of style SINGLE is not available until API version 2.20. You are currently using API version 2.16.",
}

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Looks good, though I think that if you wanted to save some diff you could have the new argument be full_message and keep message as the kwarg-only one

@SyntaxColoring SyntaxColoring merged commit d05dc2e into chore_release-8.0.0 Aug 20, 2024
62 checks passed
@SyntaxColoring SyntaxColoring deleted the fix_error_messages branch August 20, 2024 16:49
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.

2 participants