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

Feature/add validation endpoint #39

Merged
merged 9 commits into from
Jun 10, 2020
Merged

Conversation

csc-jm-zz
Copy link

@csc-jm-zz csc-jm-zz commented Jun 10, 2020

Description

PR includes an endpoint for XML validation that responds with a json object that includes boolean value for key "isValid" and a text string for the reason of failure if xml is invalid. Whether or not json response was required for this operation, I found responding with json to be substantially faster for whatever reason when xml was invalid against schema.

Related issues

Fixes #22

Type of change

  • New feature (non-breaking change which adds functionality)

Changes Made

  • Adds HTTP method for xml validation

Testing

  • Unit Tests

@csc-jm-zz csc-jm-zz changed the base branch from master to develop June 10, 2020 12:00
@csc-jm-zz csc-jm-zz marked this pull request as ready for review June 10, 2020 12:00
@csc-jm-zz csc-jm-zz requested review from otahontas and blankdots June 10, 2020 12:00
Copy link
Contributor

@otahontas otahontas left a comment

Choose a reason for hiding this comment

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

This looks fine to me, code is also easy to follow. Couple of things to note:

  • endpoint now takes any amount of xml files and validates them. Maybe response should have "areValid" or either restrict its usage to handling only one xml file?
  • I don't think adding that much inline comments is necessary, that validation part is quite clear now

Copy link
Contributor

@blankdots blankdots left a comment

Choose a reason for hiding this comment

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

small changes other than that LGTM

metadata_backend/api/handlers.py Outdated Show resolved Hide resolved
metadata_backend/api/handlers.py Outdated Show resolved Hide resolved
csc-jm and others added 4 commits June 10, 2020 16:53
Co-authored-by: Stefan Negru <[email protected]>
…tadata-submitter into feature/add-validation-endpoint

merges fixes made in remote and local repo
@csc-jm-zz
Copy link
Author

requested changes made

Copy link
Contributor

@otahontas otahontas left a comment

Choose a reason for hiding this comment

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

Just realized we need to do that len check also for get_object, so someone could refactor it to_extract_xml_upload in the future

regarding this PR, everything looks fine now! I'll test how the validation works from frontend at some point.

@blankdots blankdots merged commit 82a2dcc into develop Jun 10, 2020
@blankdots blankdots deleted the feature/add-validation-endpoint branch June 10, 2020 17:43
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.

Add validation endpoint
4 participants