Skip to content

Support for the federation extension in Jupyter visual outputs #771

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+111 −35
Open

Support for the federation extension in Jupyter visual outputs #771

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
6 changes: 4 additions & 2 deletions openeo/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,9 @@ class CollectionMetadata(CubeMetadata):

"""

def __init__(self, metadata: dict, dimensions: List[Dimension] = None):
def __init__(self, metadata: dict, dimensions: List[Dimension] = None, connection = None):
Copy link
Member

Choose a reason for hiding this comment

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

I understand what you do here, but this simple addition creates an architectural a problem: it couples a container of collection metadata to a client-side connection object:

  • the connection object is a non-trivial class with quite a lot of dependencies, so this increases the risk on import cycle issues (which is already an existing problem).
    • I was also going to give the review note to give the collection a type annotation, but when you do that (and you might already have tried that), you will get in import cycle/dependency hell, which proves my point
  • We also use the CollectionMetadata class at the backend-side to manage cube/collection metadata, but there is no connection object of any kind in that context, so this argument addition is introducing a mismatch in expectations

I have to look deeper in the PR and think it through some more,
but at first sight, you just need the connection object to get the capabilities, to get listing of backends in the federation, to be passed to render_component. These are a lot of concerns I'd like to keep outside of this class. Wouldn't it be an option to instead just add an argument like, say _repr_html_extra : dict, which you then merge into data at _repr_html_ time.

Copy link
Member

Choose a reason for hiding this comment

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

Another solution would be to add layers to the class hierachy:

  • remove _repr_html_ from CollectionMetadata class (to eliminate all client-side connection concerns) so that it can be used also in backend-side contexts
  • Introduce a child class, e.g. RichCollectionMetadata or VisualCollectionMetadata, defined under the openeo.rest "namespace", which just adds _repr_html_ to provide rich Jupyter visualisation

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it improve things if we'd just pass over the capabilities object?

self._orig_metadata = metadata
self._connection = connection
if dimensions is None:
dimensions = self._parse_dimensions(self._orig_metadata)

Expand Down Expand Up @@ -626,7 +627,8 @@ def extent(self) -> dict:
return self._orig_metadata.get("extent")

def _repr_html_(self):
return render_component("collection", data=self._orig_metadata)
federation = self._connection.capabilities().ext_federation_backend_details()
Copy link
Member

Choose a reason for hiding this comment

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

this will fail with the self._connection's default value None

return render_component("collection", data=self._orig_metadata, parameters={'federation': federation})

def __str__(self) -> str:
bands = self.band_names if self.has_band_dimension() else "no bands dimension"
Expand Down
50 changes: 36 additions & 14 deletions openeo/rest/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ def list_collections(self) -> CollectionListingResponse:
# TODO: add caching #383, but reset cache on auth change #254
# TODO #677 add pagination support?
data = self.get("/collections", expected_status=200).json()
return CollectionListingResponse(response_data=data)
return CollectionListingResponse(response_data=data, connection=self)

def list_collection_ids(self) -> List[str]:
"""
Expand Down Expand Up @@ -755,7 +755,11 @@ def list_file_formats(self) -> dict:
key="file_formats",
load=lambda: self.get('/file_formats', expected_status=200).json()
)
return VisualDict("file-formats", data=formats)
federation = self.capabilities().ext_federation_backend_details()
return VisualDict("file-formats", data=formats, parameters={
'missing': formats.get("federation:missing", None),
'federation': federation
})

def list_service_types(self) -> dict:
"""
Expand All @@ -779,7 +783,8 @@ def list_udf_runtimes(self) -> dict:
key="udf_runtimes",
load=lambda: self.get('/udf_runtimes', expected_status=200).json()
)
return VisualDict("udf-runtimes", data=runtimes)
federation = self.capabilities().ext_federation_backend_details()
return VisualDict("udf-runtimes", data=runtimes, parameters={'federation': federation})

def list_services(self) -> dict:
"""
Expand All @@ -789,7 +794,12 @@ def list_services(self) -> dict:
"""
# TODO return parsed service objects
services = self.get('/services', expected_status=200).json()["services"]
return VisualList("data-table", data=services, parameters={'columns': 'services'})
federation = self.capabilities().ext_federation_backend_details()
return VisualList("data-table", data=services, parameters={
'columns': 'services',
'missing': services.get("federation:missing", None),
'federation': federation
})

def describe_collection(self, collection_id: str) -> dict:
"""
Expand All @@ -806,7 +816,8 @@ def describe_collection(self, collection_id: str) -> dict:
# TODO: duplication with `Connection.collection_metadata`: deprecate one or the other?
# TODO: add caching #383
data = self.get(f"/collections/{collection_id}", expected_status=200).json()
return VisualDict("collection", data=data)
federation = self.capabilities().ext_federation_backend_details()
return VisualDict("collection", data=data, parameters={'federation': federation})

def collection_items(
self,
Expand Down Expand Up @@ -844,11 +855,12 @@ def collection_items(
if limit is not None and limit > 0:
params['limit'] = limit

return paginate(self, url, params, lambda response, page: VisualDict("items", data = response, parameters = {'show-map': True, 'heading': 'Page {} - Items'.format(page)}))
federation = self.capabilities().ext_federation_backend_details()
return paginate(self, url, params, lambda response, page: VisualDict("items", data = response, parameters = {'show-map': True, 'heading': 'Page {} - Items'.format(page), 'federation': federation}))

def collection_metadata(self, name) -> CollectionMetadata:
# TODO: duplication with `Connection.describe_collection`: deprecate one or the other?
return CollectionMetadata(metadata=self.describe_collection(name))
return CollectionMetadata(metadata=self.describe_collection(name), connection=self)

def list_processes(self, namespace: Optional[str] = None) -> ProcessListingResponse:
"""
Expand All @@ -870,7 +882,7 @@ def list_processes(self, namespace: Optional[str] = None) -> ProcessListingRespo
)
else:
response = self.get("/processes/" + namespace, expected_status=200).json()
return ProcessListingResponse(response_data=response)
return ProcessListingResponse(response_data=response, connection=self)

def describe_process(self, id: str, namespace: Optional[str] = None) -> dict:
"""
Expand All @@ -883,9 +895,14 @@ def describe_process(self, id: str, namespace: Optional[str] = None) -> dict:
"""

processes = self.list_processes(namespace)
federation = self.capabilities().ext_federation_backend_details()
for process in processes:
if process["id"] == id:
return VisualDict("process", data=process, parameters={'show-graph': True, 'provide-download': False})
return VisualDict("process", data=process, parameters={
'show-graph': True,
'provide-download': False,
'federation': federation
})

raise OpenEoClientException("Process does not exist.")

Expand All @@ -912,7 +929,7 @@ def list_jobs(self, limit: Union[int, None] = 100) -> JobListingResponse:
# TODO: Parse the result so that Job classes returned?
# TODO: when pagination is enabled: how to expose link to next page?
resp = self.get("/jobs", params={"limit": limit}, expected_status=200).json()
return JobListingResponse(response_data=resp)
return JobListingResponse(response_data=resp, connection=self)

def assert_user_defined_process_support(self):
"""
Expand Down Expand Up @@ -975,7 +992,7 @@ def list_user_defined_processes(self) -> ProcessListingResponse:
# TODO #677 add pagination support?
self.assert_user_defined_process_support()
data = self.get("/process_graphs", expected_status=200).json()
return ProcessListingResponse(response_data=data)
return ProcessListingResponse(response_data=data, connection=self)

def user_defined_process(self, user_defined_process_id: str) -> RESTUserDefinedProcess:
"""
Expand Down Expand Up @@ -1475,9 +1492,14 @@ def list_files(self) -> List[UserFile]:

:return: List of the user-uploaded files.
"""
files = self.get('/files', expected_status=200).json()['files']
files = [UserFile.from_metadata(metadata=f, connection=self) for f in files]
return VisualList("data-table", data=files, parameters={'columns': 'files'})
data = self.get('/files', expected_status=200).json()
files = [UserFile.from_metadata(metadata=f, connection=self) for f in data.get('files', [])]
federation = self.capabilities().ext_federation_backend_details()
return VisualList("data-table", data=files, parameters={
'columns': 'files',
'missing': data.get('federation:missing', None),
'federation': federation,
})

def get_file(
self, path: Union[str, PurePosixPath], metadata: Optional[dict] = None
Expand Down
9 changes: 6 additions & 3 deletions openeo/rest/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ def __repr__(self):

def _repr_html_(self):
data = self.describe()
currency = self.connection.capabilities().currency()
return render_component('job', data=data, parameters={'currency': currency})
capabilities = self.connection.capabilities()
return render_component('job', data=data, parameters={
'currency': capabilities.currency(),
'federation': capabilities.ext_federation_backend_details(),
})

@openeo_endpoint("GET /jobs/{job_id}")
def describe(self) -> dict:
Expand Down Expand Up @@ -217,7 +220,7 @@ def logs(self, offset: Optional[str] = None, level: Union[str, int, None] = None
if level is not None:
params["level"] = log_level_name(level)
response_data = self.connection.get(url, params=params, expected_status=200).json()
return LogsResponse(response_data=response_data, log_level=level)
return LogsResponse(response_data=response_data, log_level=level, connection=self.connection)

@deprecated("Use start_and_wait instead", version="0.39.0")
def run_synchronous(
Expand Down
65 changes: 53 additions & 12 deletions openeo/rest/models/general.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,33 @@ class CollectionListingResponse(list):
but now also provides methods/properties to access additional response data.

:param response_data: response data from a ``GET /collections`` request
:param connection: optional connection object to use for federation extension

.. seealso:: :py:meth:`openeo.rest.connection.Connection.list_collections()`

.. versionadded:: 0.38.0
"""

__slots__ = ["_data"]
__slots__ = ["_data", "_connection"]

def __init__(self, response_data: dict):
def __init__(self, response_data: dict, connection = None):
self._data = response_data
self._connection = connection
# Mimic original list of collection metadata dictionaries
super().__init__(response_data["collections"])

self.ext_federation_missing(auto_warn=True)

def _repr_html_(self):
return render_component(component="collections", data=self)
federation = self._connection.capabilities().ext_federation_backend_details() if self._connection else None
return render_component(
component="collections",
data=self,
parameters={
"missing": self.ext_federation_missing(),
"federation": federation,
}
)

@property
def links(self) -> List[Link]:
Expand Down Expand Up @@ -94,24 +104,34 @@ class ProcessListingResponse(list):
but now also provides methods/properties to access additional response data.

:param response_data: response data from a ``GET /processes`` request
:param connection: optional connection object to use for federation extension

.. seealso:: :py:meth:`openeo.rest.connection.Connection.list_processes()`

.. versionadded:: 0.38.0
"""

__slots__ = ["_data"]
__slots__ = ["_data", "_connection"]

def __init__(self, response_data: dict):
def __init__(self, response_data: dict, connection = None):
self._data = response_data
self._connection = connection
# Mimic original list of process metadata dictionaries
super().__init__(response_data["processes"])

self.ext_federation_missing(auto_warn=True)

def _repr_html_(self):
federation = self._connection.capabilities().ext_federation_backend_details() if self._connection else None
return render_component(
component="processes", data=self, parameters={"show-graph": True, "provide-download": False}
component="processes",
data=self,
parameters={
"show-graph": True,
"provide-download": False,
"missing": self.ext_federation_missing(),
"federation": federation,
}
)

@property
Expand Down Expand Up @@ -148,23 +168,34 @@ class JobListingResponse(list):
but now also provides methods/properties to access additional response data.

:param response_data: response data from a ``GET /jobs`` request
:param connection: optional connection object to use for federation extension

.. seealso:: :py:meth:`openeo.rest.connection.Connection.list_jobs()`

.. versionadded:: 0.38.0
"""

__slots__ = ["_data"]
__slots__ = ["_data", "_connection"]

def __init__(self, response_data: dict):
def __init__(self, response_data: dict, connection = None):
self._data = response_data
self._connection = connection
# Mimic original list of process metadata dictionaries
super().__init__(response_data["jobs"])

self.ext_federation_missing(auto_warn=True)

def _repr_html_(self):
return render_component(component="data-table", data=self, parameters={"columns": "jobs"})
federation = self._connection.capabilities().ext_federation_backend_details() if self._connection else None
return render_component(
component="data-table",
data=self,
parameters={
"columns": "jobs",
"missing": self.ext_federation_missing(),
"federation": federation,
}
)

@property
def links(self) -> List[Link]:
Expand Down Expand Up @@ -202,17 +233,19 @@ class LogsResponse(list):

:param response_data: response data from a ``GET /jobs/{job_id}/logs``
or ``GET /services/{service_id}/logs`` request.
:param connection: optional connection object to use for federation extension

.. seealso:: :py:meth:`~openeo.rest.job.BatchJob.logs()`
and :py:meth:`~openeo.rest.service.Service.logs()`

.. versionadded:: 0.38.0
"""

__slots__ = ["_data"]
__slots__ = ["_data", "_connection"]

def __init__(self, response_data: dict, *, log_level: Optional[str] = None):
def __init__(self, response_data: dict, *, log_level: Optional[str] = None, connection = None):
self._data = response_data
self._connection = connection

logs = response_data.get("logs", [])
# Extra client-side level filtering (in case the back-end does not support that)
Expand All @@ -237,7 +270,15 @@ def accept_level(level: str) -> bool:
self.ext_federation_missing(auto_warn=True)

def _repr_html_(self):
return render_component(component="logs", data=self)
federation = self._connection.capabilities().ext_federation_backend_details() if self._connection else None
return render_component(
component="logs",
data=self,
parameters={
"missing": self.ext_federation_missing(),
"federation": federation,
}
)

@property
def logs(self) -> List[LogEntry]:
Expand Down
9 changes: 6 additions & 3 deletions openeo/rest/service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ def __repr__(self):

def _repr_html_(self):
data = self.describe_service()
currency = self.connection.capabilities().currency()
return VisualDict('service', data = data, parameters = {'currency': currency})
capabilities = self.connection.capabilities()
return VisualDict('service', data = data, parameters = {
'currency': capabilities.currency(),
'federation': capabilities.ext_federation_backend_details(),
})

def describe_service(self):
""" Get all information about a secondary web service."""
Expand All @@ -52,4 +55,4 @@ def logs(self, offset: Optional[str] = None, level: Union[str, int, None] = None
if level is not None:
params["level"] = log_level_name(level)
response_data = self.connection.get(url, params=params, expected_status=200).json()
return LogsResponse(response_data=response_data, log_level=level)
return LogsResponse(response_data=response_data, log_level=level, connection=self.connection)
7 changes: 6 additions & 1 deletion openeo/rest/udp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,12 @@ def __init__(self, user_defined_process_id: str, connection: Connection):

def _repr_html_(self):
process = self.describe()
return render_component('process', data=process, parameters = {'show-graph': True, 'provide-download': False})
federation = self._connection.capabilities().ext_federation_backend_details()
return render_component('process', data=process, parameters = {
'show-graph': True,
'provide-download': False,
'federation': federation,
})

def store(
self,
Expand Down