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

Add database schema and validation for simulation logs #2048

Closed
jonrkarr opened this issue Jan 26, 2021 · 7 comments · Fixed by #3361
Closed

Add database schema and validation for simulation logs #2048

jonrkarr opened this issue Jan 26, 2021 · 7 comments · Fixed by #3361
Assignees
Labels
enhancement New feature or request

Comments

@jonrkarr
Copy link
Member

An endpoint analogous to the simulator/validate endpoint would be useful. Then we can use this to create a test case within the simulators test suite which checks that simulation tools produce valid logs.

@jonrkarr jonrkarr added the enhancement New feature or request label Jan 26, 2021
@bilalshaikh42
Copy link
Member

Currently the database is not validating the structure of the logs at all, assuming that the simulators will be producing a compliant output. This is similar to the results endpoints.

We could have the database perform this validation, but we would then need to define the database schemas for the log components

@jonrkarr
Copy link
Member Author

I agree with assuming that simulators produce valid results. HDF5 is quite different than JSON/YAML, and the results are heavily validated by the test suite. Results are the basis for most of the test cases.

I could also put validation for logs in biosimulators-test-suite. This could be done using the JSON schema version of the schema for the logs. But I think its easier to implement that in this repo, and treat this repo as holding the primary definition of the log format. This consolidates the definition of all JSON schemas and their validation into this repo.

@bilalshaikh42
Copy link
Member

Should this be closed/ moved to the simulators repo?

@jonrkarr jonrkarr reopened this Feb 26, 2021
@jonrkarr jonrkarr reopened this Feb 26, 2021
@jonrkarr
Copy link
Member Author

I think it would be best to implement a database schema for this. The test suite can check if simulation tools produce valid logs for a few examples, but its difficult to verify that a simulation tool will always produce valid logs.

I can make the test suite use the JSONSchema description of the schema to check that simulation tools produce valid logs (for a small set of examples)

@jonrkarr jonrkarr changed the title Add an endpoint to the dispatch API to validate a simulation log Add database schema for simulation logs Feb 26, 2021
@jonrkarr
Copy link
Member Author

I looked into using the JSONSchema version of the OpenAPI spec to validate that simulators produce valid logs within the simulators test suite. This could be done, but the JSONSchema doesn't recognize nullable. Another way to look at it is that the OpenAPI spec isn't being translated into a 100% compatible JSONSchema due to the fact that the OpenAPI spec is broader than JSONschema.

One option is for us to explicitly define null as a valid type everywhere we use nullable = true in the definition of our NestJS/Swagger API schemas. Then our OpenAPI spec could likely be translated into a functionally equalivalent JSONSchema, which we could use to validate simulators in the simulators test suite.

I think the better path is:

  • Define a database schema for execution logs
  • Use this schema to provide a validation endpoint similar to the simulator specs validation endpoint

@bilalshaikh42
Copy link
Member

bilalshaikh42 commented Feb 28, 2021 via email

@jonrkarr
Copy link
Member Author

I think the next versions of OpenAPI (3.1) and JSON Schema (Draft 4) are supposed to be compatible.

The current version of JSON Schema supports null type, but it doesn't recognize nullable. The library we're using to convert OpenAPI to JSON Schema (https://github.com/openapi-contrib/openapi-schema-to-json-schema) is supposed to convert nullable to oneOf(..., {"type": "null"}), but this doesn't happen. I haven't inspected carefully. It could be that the OpenAPI specification doesn't have nullable everywhere we expect, which prevents the OpenAPI spec from being automatically converted to a JSON Schema as we expect.

If we explored this, we could probably figure out how to get our dependencies to generate an OpenAPI spec with nullable=True where we expect and then hopefully this would get converted to JSON Schema as we expect.

@jonrkarr jonrkarr added future Features to implement in future and removed enhancement New feature or request labels Sep 28, 2021
@jonrkarr jonrkarr added enhancement New feature or request and removed future Features to implement in future labels Oct 22, 2021
@jonrkarr jonrkarr assigned jonrkarr and unassigned bilalshaikh42 Oct 22, 2021
@jonrkarr jonrkarr changed the title Add database schema for simulation logs Add database schema and validation for simulation logs Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants