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.

Sign up for GitHub

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

Jump to bottom

Handle exceptions in Vardef client #75

Merged
merged 34 commits into from
Dec 13, 2024
Merged

Handle exceptions in Vardef client #75

merged 34 commits into from
Dec 13, 2024

Conversation

tilen1976
Copy link
Contributor

@tilen1976 tilen1976 commented Dec 12, 2024 •
edited by mmwinther
Loading

Add exception class VardefClientException:

  • Response body as argument
  • Handles two formats of Json Problem:
    • Constraint violation (with list of violations)
    • Simple Json Problem
  • Returns clear exception message with status and detail/message
  • Method handles exceptions gracefully and returns default text for status/detail/messages
  • Add decorator method:
    • Handle exceptions in the background
    • Catch exceptions from Api and raise VardefClientException

Add decorator on Vardef methods

@tilen1976
add create draft exception example
a079340
@tilen1976
Add test invalid unit types - correct unittypes happy path
7e8abba
@tilen1976
change exception
e29b083
@tilen1976
add test exception message
0bf1efc
@tilen1976
assert 404 not working correctly
d7adca5
@tilen1976
add new open api file
981b682
@tilen1976
specify exception
9eef1d6
@tilen1976
linting
7391582
@tilen1976
update short name
dff02c4
@tilen1976
Remove bad request example - add not found variable definition exampl…
ad7d303 
…e and test
@tilen1976
last edition not found
6d2c333
@tilen1976
remove fixture
07144c2
@tilen1976
updated open api spec
dfcd030
@tilen1976
Test custom exception message
63a93ad
@tilen1976
handle constraint violations
62e52f9
@tilen1976
update open api file
f002d13
@tilen1976
update open api file
f0a26a3
@tilen1976
add tests for constraint violations
01462a4
@tilen1976
add tests for missing elements
1c32418
@tilen1976
update open api file with new excptions
65424a5
@tilen1976
update tests with new exception structure
ba6e735
@tilen1976
start of decorator for vardef client exception handling
3908cc3
@tilen1976
Merge branch 'main' into feat/vardef-client-add-tests-exceptions
ffa4018
@tilen1976
decorator ok
6045bb8
@tilen1976
set temp type ignore
1504eee
@tilen1976
rename test
180984a
@tilen1976 tilen1976 marked this pull request as ready for review December 12, 2024 13:31
@tilen1976 tilen1976 requested a review from a team as a code owner December 12, 2024 13:31
mmwinther
mmwinther requested changes Dec 12, 2024
View reviewed changes
Copy link
Member

@mmwinther mmwinther left a comment

Choose a reason for hiding this comment

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

Very elegant implementation and great test coverage 😄 I think there are a couple of things which could be simplified.

src/dapla_metadata/variable_definitions/exceptions.py Outdated Show resolved Hide resolved
src/dapla_metadata/variable_definitions/exceptions.py Outdated Show resolved Hide resolved
src/dapla_metadata/variable_definitions/exceptions.py Outdated Show resolved Hide resolved
src/dapla_metadata/variable_definitions/exceptions.py Outdated Show resolved Hide resolved
tests/variable_definitions/vardef_client/api/test_error_path.py Outdated Show resolved Hide resolved
mmwinther
mmwinther approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@mmwinther mmwinther left a comment

Choose a reason for hiding this comment

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

Nice work 👍

@tilen1976 tilen1976 merged commit 82a1353 into main Dec 13, 2024
12 of 13 checks passed
@tilen1976 tilen1976 deleted the feat/vardef-client-add-tests-exceptions branch December 13, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmwinther mmwinther mmwinther approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tilen1976 @mmwinther