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

Test Search API on ISIS ICAT #319

Closed
1 task
MRichards99 opened this issue Feb 3, 2022 · 6 comments
Closed
1 task

Test Search API on ISIS ICAT #319

MRichards99 opened this issue Feb 3, 2022 · 6 comments
Assignees
Labels
expands-search-api Issues relating to the ExPaNDS Search API section of this repo

Comments

@MRichards99
Copy link
Collaborator

Description:
Once the endpoints have been finished and merged, we should probably point the search API at ISIS ICAT dev and do a bit of manual testing. Send a request for each type of endpoint (for each entity too) to check they all work. Send requests with some where and include filters to ensure conditions work and related entities return as expected.

Acceptance criteria:

  • Test search API when pointed to ISIS ICAT
@MRichards99 MRichards99 added the expands-search-api Issues relating to the ExPaNDS Search API section of this repo label Feb 3, 2022
@MRichards99
Copy link
Collaborator Author

#314 will fix various issues I've noticed on /datasets and /instruments (due to ISIS having no PIDs on these entities)

@VKTB
Copy link
Contributor

VKTB commented Feb 18, 2022

Sending the following request:

https://datagateway-dev.isis.stfc.ac.uk/search-api/documents?filter={ "include": [ { "relation": "members", "scope": { "where": { "role": { "neq": "principal_experimenter" } }, "include": [ { "relation": "person" } ] } }, { "relation": "datasets" } ], "limit": 5 }

returns 500 INTERNAL SERVER ERROR The server encountered an internal error and was unable to complete your request. Either the server is overloaded or there is an error in the application.

That same request returns data fine when I run it against my local Search API instance which points to SciGateway preprod ICAT.

@MRichards99
Copy link
Collaborator Author

@VKTB this looks to be caused by the differences between the ICAT schema and the PaNOSC data model. By using limit and skip to target certain records, that request does work. The following request (the same as yours but has a limit of 1 and skip of 2) does work:

http://{{datagateway-api}}/search-api/documents?filter={"limit": 10}&filter={ "include": [ { "relation": "members", "scope": { "where": { "role": { "neq": "principal_experimenter" } }, "include": [ { "relation": "person" } ] } }, { "relation": "datasets" } ], "limit": 1, "skip": 2}

However, if you use a skip of 3, you get an error. Looking at my logs (I'm using my local instance pointing at ISIS dev ICAT), there's a ValidationError:

Traceback (most recent call last):
  File "/root/seg_dev/datagateway-api/datagateway_api/src/search_api/helpers.py", line 38, in wrapper_error_handling
    return method(*args, **kwargs)
  File "/root/seg_dev/datagateway-api/datagateway_api/src/resources/search_api_endpoints.py", line 34, in get
    return get_search(entity_name, filters), 200
  File "/root/seg_dev/datagateway-api/datagateway_api/src/search_api/session_handler.py", line 38, in wrapper_client_manager
    return method(*args, **kwargs)
  File "/root/seg_dev/datagateway-api/datagateway_api/src/search_api/helpers.py", line 104, in get_search
    panosc_record = panosc_model.from_icat(icat_data, entity_relations).json(
  File "/root/seg_dev/datagateway-api/datagateway_api/src/search_api/models.py", line 244, in from_icat
    return super(Document, cls).from_icat(icat_data, required_related_fields)
  File "/root/seg_dev/datagateway-api/datagateway_api/src/search_api/models.py", line 142, in from_icat
    raise ValidationError(errors=[error_wrapper], model=cls)
pydantic.error_wrappers.ValidationError: 1 validation error for Document
datasets
  field required (type=type_error)

This looks as if including datasets on the document doesn't work. Or in other words, that particular document doesn't have a dataset linked to it. If you look at the ICAT schema for Investigation, the relationship between itself and dataset isn't mandatory, explaining how this can happen.

I think you've tried to deal with this by adding the _related_fields_with_min_cardinality_one attributes in each of the PaNOSC entities in models.py. For some reason, I actually get a 400 on my instance (a 500 exists on the ISIS dev instance because it doesn't have the error handling stuff in). Perhaps we could 'more gracefully' deal with this error by making it return a 500 and a more detailed error message (see below). Beyond that, I think its just a reality of the two data models that sometimes differ.

Response from a broken request on my instance:

{
    "error": {
        "message": "Bad request",
        "name": "BadRequestError",
        "statusCode": 400
    }
}

Either one of us can look at improving this, it just depends on who needs more work first.

@VKTB
Copy link
Contributor

VKTB commented Feb 21, 2022

Sending the following request (which is basically the Query datasets where the photon energy range is 880-990 eV example query):

https://datagateway-dev.isis.stfc.ac.uk/search-api/datasets?filter={ "include": [ { "relation": "parameters", "scope": { "where": { "and": [ { "name": "photon_energy" }, { "value": { "between": [880, 990] } }, { "unit": "eV" } ] } } } ] }

returns 400 BAD REQUEST issubclass() arg 1 must be a class.

The same error is returned by my local Search API instance which points to SciGateway preprod ICAT.

Edit:
The same error is returned for the following requests (which are basically the Query datasets with a solid sample or copper and Query datasets where temperature is below 80°C example queries):

https://datagateway-dev.isis.stfc.ac.uk/search-api/datasets?filter={ "include": [ { "relation": "parameters", "scope": { "where": { "or": [ { "and": [ { "name": "sample_state" }, { "value": "solid" } ] }, { "and": [ { "name": "chemical_formula" }, { "value": "Cu" } ] } ] } } } ] }
https://datagateway-dev.isis.stfc.ac.uk/search-api/datasets?filter={ "include": [ { "relation": "parameters", "scope": { "where": { "and": [ { "name": "temperature" }, { "value": { "lt": 80 } }, { "unit": "celsius" } ] } } } ] }

@VKTB
Copy link
Contributor

VKTB commented Feb 21, 2022

Replying to #319 (comment)

@MRichards99 Thanks for looking into that. I agree, we should return a 500 error instead when it is an issue on our side.

I did not realise that the Investigation entity in ICAT can have a zero or many Datasets but that will cause 500 errors in some cases which is not good. Maybe we should raise this and check if the cardinality of the datasets Document field should be 0, * instead?

@MRichards99
Copy link
Collaborator Author

Closing this issue as we haven't heard anything back about the datasets relationship issue. As a result, we don't intend on making any changes to the data model at this stage. If we are asked to change the data model in the future, it should be a simple change to make and test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expands-search-api Issues relating to the ExPaNDS Search API section of this repo
Projects
None yet
Development

No branches or pull requests

2 participants