-
Notifications
You must be signed in to change notification settings - Fork 68
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
[SYNPY-1322] Object Orientated Programming Interfaces #1013
Changes from 22 commits
625b0ba
6105a71
cebe138
89cd7c7
40250fb
dd0eeb0
03cf453
dcc29f0
77a8d6b
b27cf12
244c69f
55ee852
65a1c70
57da4e1
5bdf4c4
1a11b17
f988b5c
5d5f9f5
6b5b9af
86a474e
dfbddcc
6b74371
84a5369
d370b46
3aec4ff
93bacca
bbe905f
410de22
be9f193
cc049b0
53e4c22
8fef247
a19d77a
377df74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# These are all of the models that are used by the Synapse client. | ||
from .annotations import set_annotations | ||
|
||
|
||
__all__ = [ | ||
"set_annotations", | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
""" | ||
The purpose of this module is to provide any functions that are needed to interact with | ||
annotations that are not cleanly provided by the synapseclient library. | ||
""" | ||
import json | ||
|
||
from dataclasses import asdict | ||
|
||
from typing import TYPE_CHECKING, Optional | ||
from synapseclient import Synapse | ||
from opentelemetry import context | ||
|
||
if TYPE_CHECKING: | ||
from synapseclient.models import Annotations | ||
|
||
|
||
def set_annotations( | ||
annotations: "Annotations", | ||
synapse_client: Optional[Synapse] = None, | ||
opentelemetry_context: Optional[context.Context] = None, | ||
): | ||
"""Call to synapse and set the annotations for the given input. | ||
|
||
:param annotations: The annotations to set. This is expected to have the id, etag, and annotations filled in. | ||
:param synapse_client: If not passed in or None this will use the last client from the `.login()` method. | ||
:param opentelemetry_context: OpenTelemetry context to propogate to this function to use for tracing. Used | ||
cases where concurrent operations need to be linked to parent spans. | ||
:return: _description_ | ||
""" | ||
annotations_dict = asdict(annotations) | ||
|
||
# TODO: Is there a more elegant way to handle this - This is essentially being used | ||
# TODO: to remove any fields that are not expected by the REST API. | ||
filtered_dict = { | ||
k: v for k, v in annotations_dict.items() if v is not None and k != "is_loaded" | ||
} | ||
|
||
# TODO: This `restPUT` returns back a dict (or string) - Could we use: | ||
# TODO: https://github.com/konradhalas/dacite to convert the dict to an object? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do like the look of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure yet - I'll have to give it some more thought. One of the things we will need to do it at least have a thin translation layer between the REST api and the dataclasses because the names we are giving them in the python client are snake_case, vs the REST api is all in camelCase. |
||
return Synapse.get_client(synapse_client=synapse_client).restPUT( | ||
f"/entity/{annotations.id}/annotations2", | ||
body=json.dumps(filtered_dict), | ||
opentelemetry_context=opentelemetry_context, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is another dataclass library that adds some more functionality. Its
asdict
function has anexclude
argument that you can pass to leave out class attributes you don't want in your dictionary:You'd still be hard-coding the excluded values though. Not sure of any other ways aside from implementing an
asdict
method specific to the class that excludes attributes not used in the API.Edit: if you used a base class you could potentially implement an
asdict
function that could be reused across all extending classesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea - but i'm starting to think in the other direction now. What I mean by this is specify only those things I want to include and have a dataclass -> json/dict mapping process to format the input for the REST API as the API is expecting.