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

Simplify json error reporting #3286

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Simplify json error reporting #3286

merged 2 commits into from
Mar 25, 2024

Conversation

amickan
Copy link
Contributor

@amickan amickan commented Mar 25, 2024

Json validation errors used to include the instance (the submitted json), the schema and the error. This changes the reporting to only print the error message without the schema and instance. This is a temporary fix until this is implemented: python-jsonschema/jsonschema#1218

It also standardizes the message for FILE_COPY_STATUS notifications, opting for the less verbose version.

@amickan amickan marked this pull request as ready for review March 25, 2024 10:27
@amickan amickan requested a review from jmsmkn as a code owner March 25, 2024 10:27
@amickan
Copy link
Contributor Author

amickan commented Mar 25, 2024

Some of the tests that involve checking the returned json validation error currently don't pass. I wanted to get feedback on the general approach first before I change the tests. Maybe we decide not to change anything and wait for the feature request to be implemented.

@jmsmkn
Copy link
Member

jmsmkn commented Mar 25, 2024

Yes the approach is fine for now. Would be happy for you to work on fixing this upstream if you can.

@amickan
Copy link
Contributor Author

amickan commented Mar 25, 2024

Would be happy for you to work on fixing this upstream if you can.

I think I will wait this one out a little longer. From the conversation on the issue it sounds like the person who proposed it has a pretty clear idea of how to implement it already (whereas I don't, yet) and I have other things higher up on my priority list at the moment. I might give it a shot later though, if it hasn't been tackled by then.

@amickan amickan merged commit 9b4f113 into main Mar 25, 2024
8 checks passed
@amickan amickan deleted the json_error_reporting branch March 25, 2024 14:50
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