Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Expose record_type for responses in graphql #196

Closed
wants to merge 1 commit into from
Closed

Expose record_type for responses in graphql #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/ert_storage/graphql/responses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from typing import List, Optional, TYPE_CHECKING
from enum import Enum
from typing import Any, List, Optional, TYPE_CHECKING
import graphene as gr

from ert_storage.ext.graphene_sqlalchemy import SQLAlchemyObjectType
Expand All @@ -14,6 +15,10 @@ class Meta:
model = ds.Record

name = gr.String()
record_type = gr.Enum.from_enum(ds.RecordType)

def resolve_name(root: ds.Record, info: "ResolveInfo") -> str:
return root.name

def resolve_record_type(root: ds.Record, info: "ResolveInfo") -> Enum:
return root.record_info.record_type
2 changes: 2 additions & 0 deletions tests/integration/gql/test_responses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
ensemble(id: $id) {
uniqueResponses {
name
recordType
}
}
}
Expand Down Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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? 🤔

Copy link
Contributor Author

@xjules xjules Jan 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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


# retrieve all responses and realizations
r = client.gql_execute(GET_ALL_RESPONSES, variable_values={"id": ensemble_id})
Expand Down