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

Refactor error handling for scope cloning #1458

Conversation

KaczuH
Copy link
Contributor

@KaczuH KaczuH commented Aug 14, 2024

Changed from manual status code checking to using raise_for_status() for better exception handling. This improves code readability and leverages built-in exception features of the requests library.

Fixes #

Checklist:

  • My code follows the pykechain style
    • I added type hinting to all functions
    • I added documentation for all public functions
    • I locally used tox to check for dists and docs errors
    • My code passes the pep8 and flake8 linting checks and no warnings
    • I removed unused imports, using Code > Optimize Imports in PyCharm
    • I used black to format the new code
  • I have added tests that prove my fix is effective or that my feature works
    • I committed fresh test cassettes
    • I have proper test coverage (don't decline the coverage of the test)
  • I asked another teammate to review the code
  • I update the Changelog.md with the appropriate changes.

Kamil Niski added 3 commits August 15, 2024 00:26
Changed from manual status code checking to using `raise_for_status()` for better exception handling. This improves code readability and leverages built-in exception features of the requests library.
Changed the HTTP status code check from `err.response` to `response` in the error handling block. This ensures that the correct response object is used when raising a `ForbiddenError` for a forbidden scope clone attempt.
This commit enhances the scope cloning logic by checking for specific response status codes. It ensures that the function returns the appropriate response for both `created` and `accepted` statuses while raising an error for unexpected responses.
Copy link
Member

@jberends jberends left a comment

Choose a reason for hiding this comment

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

aaaah this would work too and more robust!

jberends and others added 2 commits August 15, 2024 08:00
Updated the changelog to reflect that a 202 Accepted HTTP response is now also accepted for the scope clone operation. This change is tracked under issue KE-works#1459.
@jberends jberends enabled auto-merge (squash) August 15, 2024 08:24
@jberends jberends disabled auto-merge August 15, 2024 08:26
@jberends jberends merged commit f8393e4 into KE-works:main Aug 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

scope clone now also may return 202 accepted HTTP code when cloning scope in async
2 participants