Expose record_type for responses in graphql #196
Conversation
xjules
force-pushed
the
add_record_type_gql
branch
2 times, most recently
from
50853fb to
d571763
Compare
|
Test this please! |
| @@ -73,6 +74,7 @@ def test_get_gql_response(client, create_experiment, create_ensemble): | |||
| r = client.gql_execute(GET_UNIQUE_RESPONSES, variable_values={"id": ensemble_id}) | |||
| for response in r["data"]["ensemble"]["uniqueResponses"]: | |||
| assert response["name"] in RESPONSE_NAMES | |||
| assert response["recordType"] == "F64_MATRIX" | |||
There was a problem hiding this comment.
Without knowing the details of how F64_MATRIX is used in ert-storage, I feel like we are leaking implementation details from storage here when we expose the F64_MATRIX as part of the API. Especially in combination with equinor/ert#2782, where we use this information without any abstraction in the engine module.
Also why are we only exposing it through the graphql API? 🤔
There was a problem hiding this comment.
Without knowing the details of how
F64_MATRIXis used inert-storage, I feel like we are leaking implementation details from storage here when we expose theF64_MATRIXas part of the API. Especially in combination with equinor/ert#2782, where we use this information without any abstraction in the engine module.
I've created an issue that suggest a better naming of record_types #198
Also why are we only exposing it through the graphql API? 🤔
I agree, but how such a rest endpoint would look like? Are we to support each record in a similar way to /userdata? Graphql provides an easy way to link record_type attribute to each query containing response attribute implicitly...
But we can create an issue on this I think.
There was a problem hiding this comment.
Also why are we only exposing it through the graphql API? 🤔
We've decided that ERT Storage is a GraphQL API with a REST API only for uploading and downloading records. There is difficulty in implementing this at the moment, however, seeing as it requires simultaneous PRs in ERT and ERT Storage. I have made a PR in ERT asking to put an upper version limit so that this may be implemented: equinor/ert#2815
xjules
force-pushed
the
add_record_type_gql
branch
from
d571763 to
f1b8c61
Compare
|
Currently only |
|
As mentioned above, there is no need to check the type of responses therefore closing this one. |
Issue
Relates to equinor/ert#2715
Approach
Expose record_type parameter in graphql and integrate in test.