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

[SYNPY-1322] Object Orientated Programming Interfaces #1013

Merged
merged 34 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
625b0ba
OTEL Additions to what is/isn't a span
BryanFauble Nov 10, 2023
6105a71
>= dependencies
BryanFauble Nov 10, 2023
cebe138
Add in document and script around benchmarking
BryanFauble Nov 14, 2023
89cd7c7
Code review feedback
BryanFauble Nov 16, 2023
40250fb
Link to tutorial section
BryanFauble Nov 16, 2023
dd0eeb0
Adjust OTEL dependencies
BryanFauble Nov 16, 2023
03cf453
Mark test as flaky
BryanFauble Nov 16, 2023
dcc29f0
Publish branch to remote
BryanFauble Nov 16, 2023
77a8d6b
Import File earlier
BryanFauble Nov 16, 2023
b27cf12
Merge branch 'develop' into SYNPY-1322-OOP-POC
BryanFauble Nov 16, 2023
244c69f
Support for annotations,cache syn client instance
BryanFauble Nov 20, 2023
55ee852
Remove httpx
BryanFauble Nov 20, 2023
65a1c70
Merge branch 'develop' into SYNPY-1322-OOP-POC
BryanFauble Nov 28, 2023
57da4e1
Adding support for Table
BryanFauble Nov 30, 2023
5bdf4c4
Adding delete row note
BryanFauble Nov 30, 2023
1a11b17
Adding delete table
BryanFauble Nov 30, 2023
f988b5c
More example scripts
BryanFauble Nov 30, 2023
5d5f9f5
Remove comment
BryanFauble Nov 30, 2023
6b5b9af
Prevent creating instance for calling class method
BryanFauble Nov 30, 2023
86a474e
Correcting OTEL context propogation
BryanFauble Dec 1, 2023
dfbddcc
Add note about assuming annotation type
BryanFauble Dec 1, 2023
6b74371
Merge branch 'develop' into SYNPY-1322-OOP-POC
BryanFauble Dec 7, 2023
84a5369
Adding more verbose examples for current project interface
BryanFauble Dec 8, 2023
d370b46
simplify the annotation interface (#1022)
BryanFauble Dec 12, 2023
3aec4ff
[SYNPY-1345] Migrate to mkdocstrings (#1023)
BryanFauble Dec 12, 2023
93bacca
Making sticky nav tabs per team meeting
BryanFauble Dec 12, 2023
bbe905f
Merge branch 'develop' into SYNPY-1322-OOP-POC
BryanFauble Jan 16, 2024
410de22
Remove content not needed
BryanFauble Jan 16, 2024
be9f193
Remove content that is not needed
BryanFauble Jan 16, 2024
cc049b0
Merge develop and consistency changes
BryanFauble Jan 16, 2024
53e4c22
Merge branch 'develop' into SYNPY-1322-OOP-POC
BryanFauble Jan 17, 2024
8fef247
Correct for some changes
BryanFauble Jan 17, 2024
a19d77a
Clean up doc structure
BryanFauble Jan 22, 2024
377df74
Code review updates
BryanFauble Jan 22, 2024
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: 7 additions & 0 deletions synapseclient/api/__init__.py
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",
]
39 changes: 39 additions & 0 deletions synapseclient/api/annotations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""
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

if TYPE_CHECKING:
from synapseclient.models import Annotations


def set_annotations(
annotations: "Annotations", synapse_client: Optional[Synapse] = 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.
:return: _description_
"""
annotations_dict = asdict(annotations)

# TODO: Is there a more elegant way to handle this - This is essentially being used
Copy link
Contributor

@BWMac BWMac Nov 21, 2023

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 an exclude argument that you can pass to leave out class attributes you don't want in your dictionary:

from dataclasses import dataclass
from dataclass_wizard.dumpers import asdict

@dataclass
class Foo:
    bar: str
    baz: int

foo = Foo('bar', 42)
print(asdict(foo, exclude=['baz']))

> {'bar': 'bar'}

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 classes

Copy link
Contributor Author

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.

# 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 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?
Copy link
Contributor

@BWMac BWMac Nov 21, 2023

Choose a reason for hiding this comment

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

I do like the look of dacite, but is that more efficient than just wrapping the response in the class object like we have done elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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),
)
)
33 changes: 33 additions & 0 deletions synapseclient/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ class Synapse(object):

"""

_synapse_client = None

# TODO: add additional boolean for write to disk?
@tracer.start_as_current_span("Synapse::__init__")
def __init__(
Expand Down Expand Up @@ -300,6 +302,30 @@ def _init_logger(self):
self.logger = logging.getLogger(logger_name)
logging.getLogger("py.warnings").handlers = self.logger.handlers

@classmethod
def get_client(cls, synapse_client: None) -> "Synapse":
"""
Convience function to get an instance of 'Synapse'. The latest instance created
by 'login()' or set via `set_client` will be returned.

When 'logout()' is called it will delete the instance.

:param syn: An instance of 'Synapse' or None. This is used to simplify logical checks
in cases where synapse is passed into them.
"""
if synapse_client:
return synapse_client

if not cls._synapse_client:
raise SynapseError(
"No instance has been created - Please use login() first"
)
return cls._synapse_client

@classmethod
def set_client(cls, synapse_client) -> None:
cls._synapse_client = synapse_client

@property
def max_threads(self) -> int:
return self._max_threads
Expand Down Expand Up @@ -401,6 +427,7 @@ def login(
silent: bool = False,
forced: bool = False,
authToken: str = None,
cache_client: bool = True,
):
"""
Valid combinations of login() arguments:
Expand Down Expand Up @@ -428,6 +455,9 @@ def login(
credential storage.
:param authToken: A bearer authorization token, e.g. a personal access token, can be used in lieu of a
password or apiKey
:param cache_client: Whether to cache the Synapse client object in the Synapse module. Defaults to True.
When set to True anywhere a `Synapse` object is optional you do not need to pass an
instance of `Synapse` to that function, method, or class.

**GNOME Keyring** (recommended) or **KWallet** is recommended to be installed for credential storage on
**Linux** systems.
Expand Down Expand Up @@ -515,6 +545,8 @@ def login(
else self.credentials.username
)
)
if cache_client:
Synapse().set_client(self)

def _get_config_section_dict(self, section_name: str) -> dict:
config = self.getConfigFile(self.configPath)
Expand Down Expand Up @@ -614,6 +646,7 @@ def logout(self, forgetMe: bool = False):
self.credentials.delete_from_keyring()

self.credentials = None
Synapse().set_client(None)

def invalidateAPIKey(self):
"""Invalidates authentication across all clients."""
Expand Down
3 changes: 1 addition & 2 deletions synapseclient/core/retry.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import random
import sys
import logging
from opentelemetry import trace

from synapseclient.core.logging_setup import DEBUG_LOGGER_NAME, DEFAULT_LOGGER_NAME
from synapseclient.core.utils import is_json
from synapseclient.core.dozer import doze
from opentelemetry import trace

DEFAULT_RETRIES = 3
DEFAULT_WAIT = 1
Expand All @@ -25,7 +25,6 @@
"couldn't connect to host",
"slowdown",
"try again",
"connection reset by peer",
]

# Exceptions that may be retryable. These are socket level exceptions
Expand Down
18 changes: 18 additions & 0 deletions synapseclient/models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# These are all of the models that are used by the Synapse client.
from synapseclient.models.annotations import (
Annotations,
AnnotationsValue,
AnnotationsValueType,
)
from synapseclient.models.file import File
from synapseclient.models.folder import Folder
from synapseclient.models.project import Project

__all__ = [
"File",
"Folder",
"Project",
"Annotations",
"AnnotationsValue",
"AnnotationsValueType",
]
1 change: 1 addition & 0 deletions synapseclient/models/activity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# TODO
80 changes: 80 additions & 0 deletions synapseclient/models/annotations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import asyncio

from enum import Enum
from dataclasses import dataclass
from typing import Dict, List, Optional, Union
from synapseclient.api import set_annotations
from opentelemetry import trace

from synapseclient import Synapse


tracer = trace.get_tracer("synapseclient")


class AnnotationsValueType(str, Enum):
"""The acceptable types that an annotation value can be."""

STRING = "STRING"
DOUBLE = "DOUBLE"
LONG = "LONG"
TIMESTAMP_MS = "TIMESTAMP_MS"
BOOLEAN = "BOOLEAN"


@dataclass()
class AnnotationsValue:
"""A specific type of annotation and the values that are of that type."""

type: AnnotationsValueType
# TODO: What are all the python types we are going to accept here
value: List[Union[str, bool]]


@dataclass()
class Annotations:
BryanFauble marked this conversation as resolved.
Show resolved Hide resolved
"""Annotations that can be applied to a number of Synapse resources to provide additional information."""

annotations: Dict[str, AnnotationsValue]
""" Additional metadata associated with the object. The key is the name of your
desired annotations. The value is an object containing a list of string values
(use empty list to represent no values for key) and the value type associated with
all values in the list
"""

id: Optional[str] = None
"""ID of the object to which this annotation belongs. Not required if being used as
a member variable on another class."""

etag: Optional[str] = None
""" Etag of the object to which this annotation belongs. To update an AnnotationV2,
this field must match the current etag on the object. Not required if being used as
a member variable on another class."""

is_loaded: bool = False

async def store(
self,
synapse_client: Optional[Synapse] = None,
):
"""Storing annotations to synapse."""
# TODO: Validation that id and etag are present

print(f"Storing annotations for id: {self.id}, etag: {self.etag}")
with tracer.start_as_current_span(f"Annotation_store: {self.id}"):
loop = asyncio.get_event_loop()
await loop.run_in_executor(
None,
lambda: set_annotations(
annotations=self, synapse_client=synapse_client
),
)
print(f"annotations store for {self.id} complete")
# TODO: From the returned call do we need to update anything in the root object?
return self

async def get(self):
"""Get the annotations from synapse."""
print(f"Getting annotations for id: {self.id}, etag: {self.etag}")
await asyncio.sleep(1)
self.is_loaded = True
1 change: 1 addition & 0 deletions synapseclient/models/evaluation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# TODO
100 changes: 100 additions & 0 deletions synapseclient/models/file.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import asyncio
from dataclasses import dataclass
from typing import Dict, Union
from opentelemetry import trace, context
from synapseclient.models import AnnotationsValue, Annotations

# import uuid
BryanFauble marked this conversation as resolved.
Show resolved Hide resolved

from synapseclient.entity import File as SynapseFile
from synapseclient import Synapse

from typing import Optional, TYPE_CHECKING


if TYPE_CHECKING:
from synapseclient.models import Folder, Project


tracer = trace.get_tracer("synapseclient")


@dataclass()
class File:
BryanFauble marked this conversation as resolved.
Show resolved Hide resolved
id: str
"""The unique immutable ID for this file. A new ID will be generated for new Files.
Once issued, this ID is guaranteed to never change or be re-issued"""

name: str
"""The name of this entity. Must be 256 characters or less.
Names may only contain: letters, numbers, spaces, underscores, hyphens, periods,
plus signs, apostrophes, and parentheses"""

path: str
# TODO - Should a file also have a folder, or a method that figures out the folder class?

description: Optional[str] = None
"""The description of this file. Must be 1000 characters or less."""

etag: Optional[str] = None
created_on: Optional[str] = None
modified_on: Optional[str] = None
created_by: Optional[str] = None
modified_by: Optional[str] = None
parent_id: Optional[str] = None
concrete_type: Optional[str] = None
version_number: Optional[int] = None
version_label: Optional[str] = None
version_comment: Optional[str] = None
is_latest_version: Optional[bool] = False
data_file_handle_id: Optional[str] = None
file_name_override: Optional[str] = None

annotations: Optional[Dict[str, AnnotationsValue]] = None
"""Additional metadata associated with the folder. The key is the name of your
desired annotations. The value is an object containing a list of values
(use empty list to represent no values for key) and the value type associated with
all values in the list."""

is_loaded: bool = False

# TODO: How the parent is stored/referenced needs to be thought through
async def store(
self,
parent: Union["Folder", "Project"],
synapse_client: Optional[Synapse] = None,
):
"""Storing file to synapse."""
with tracer.start_as_current_span(f"File_Store: {self.path}"):
# TODO - We need to add in some validation before the store to verify we have enough
# information to store the data

# Call synapse
loop = asyncio.get_event_loop()
synapse_file = SynapseFile(path=self.path, name=self.name, parent=parent.id)
# TODO: Propogating OTEL context is not working in this case
entity = await loop.run_in_executor(
None,
lambda: Synapse()
.get_client(synapse_client=synapse_client)
.store(obj=synapse_file, opentelemetry_context=context.get_current()),
)
print(entity)
self.id = entity.id
self.etag = entity.etag

print(f"Stored file {self.name}, id: {self.id}: {self.path}")

if self.annotations:
result = await Annotations(
id=self.id, etag=self.etag, annotations=self.annotations
).store(synapse_client=synapse_client)
print(result)

return self

async def get(self):
"""Get metadata about the folder from synapse."""
print(f"Getting file {self.name}")
await asyncio.sleep(1)
self.is_loaded = True
Loading
Loading