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: add 422 Error class and status_code to DataCiteError #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alee
Copy link

@alee alee commented Aug 3, 2024

use a defaultdict to map status codes to error classes, defaults to DataCiteServerError for status_code >= 500 and DataCiteRequestError for all others

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@mfenner mfenner self-requested a review August 6, 2024 14:59
@alee alee force-pushed the include_error_codes branch from b2ddb55 to 1015182 Compare August 13, 2024 03:19
- add 422 Unprocessable Entity error class, resolves inveniosoftware#89
- minor refactoring to use a defaultdict data structure to map status
  codes to error classes instead of an if/elif conditional chain.
- future status codes can be supported by adding a new DataCiteError
  subtype and an entry to the DataCiteErrorFactory.ERROR_CLASSES
  dictionary
- includes the status code in the Error class so client code can
  retrieve the status code from the raised Exception, resolves inveniosoftware#98

defaults to DataCiteServerError for any unhandled status_code >= 500 and
DataCiteRequestError if there is not an exact match
@tmorrell tmorrell force-pushed the include_error_codes branch from 1015182 to 41be4e5 Compare October 17, 2024 20:06
@tmorrell
Copy link
Contributor

Hi @alee. Sorry for the delay in looking at this. It looks like status_code needs to be added to all the tests in order for this improvement to go through. It also might not be a bad idea to run the tests against the live DataCite endpoint and see what the actual response is. I think a bunch of the test responses don't match what DataCite currently uses. I'll also add this to my list of things to work on, but not sure when I'll get to it.

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