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

Validation error for Sample pid field when ICAT value is None #314

Closed
1 task
VKTB opened this issue Feb 1, 2022 · 1 comment · Fixed by #324
Closed
1 task

Validation error for Sample pid field when ICAT value is None #314

VKTB opened this issue Feb 1, 2022 · 1 comment · Fixed by #324
Assignees
Labels
bug Something isn't working expands-search-api Issues relating to the ExPaNDS Search API section of this repo

Comments

@VKTB
Copy link
Contributor

VKTB commented Feb 1, 2022

Description:
Sending the following request:
http://{{datagateway-api}}/search-api/datasets?filter={"include": [{"relation": "samples"}]}

returns 500 INTERNAL SERVER ERROR:

pydantic.error_wrappers.ValidationError: 1 validation error for Sample
    pid
    field required (type=value_error.missing)

For ISIS, the Sample pid PaNOSC field maps to the Sample pid field in ICAT, however the pid values of some ICAT samples are None. These samples are the ones that are causing ValidationError because the Sample pid PaNOSC field is defined as mandatory in the Sample pydantic model which means that it does not accept a None value.

Following a chat with @louise-davies, we decided that we should set the pid at the Search API level. For example, when pid of an ICAT sample is None, we should instead take its id and set the PaNOSC pid to be pid:<id>. The decision was made based on the suggestions in icatproject/icat.server#231

Acceptance criteria:

  • Datasets with Samples should be returned
@VKTB VKTB added bug Something isn't working expands-search-api Issues relating to the ExPaNDS Search API section of this repo labels Feb 1, 2022
@VKTB
Copy link
Contributor Author

VKTB commented Feb 1, 2022

We will probably face the same issue highlighted in #308 if we are setting the pid at the Search API level.

@VKTB VKTB self-assigned this Feb 4, 2022
@VKTB VKTB linked a pull request Feb 4, 2022 that will close this issue
7 tasks
VKTB added a commit that referenced this issue Feb 8, 2022
Co-authored-by: Matthew Richards <32678030+MRichards99@users.noreply.github.com>
VKTB added a commit that referenced this issue Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working expands-search-api Issues relating to the ExPaNDS Search API section of this repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant