From 2f02858eb54ad5b4f3d9a91edc7508fc6b5e1135 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Fri, 17 Feb 2023 15:44:54 +0100 Subject: [PATCH 01/13] Draft for /files support #377 --- openeo/internal/jupyter.py | 2 ++ openeo/rest/connection.py | 13 +++++---- openeo/rest/userfile.py | 58 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 openeo/rest/userfile.py diff --git a/openeo/internal/jupyter.py b/openeo/internal/jupyter.py index 90d82f7cf..ac98588cf 100644 --- a/openeo/internal/jupyter.py +++ b/openeo/internal/jupyter.py @@ -102,6 +102,8 @@ def render_component(component: str, data = None, parameters: dict = None): # Set the data as the corresponding parameter in the Vue components key = COMPONENT_MAP.get(component, component) if data is not None: + if isinstance(data, list): + data = [(x.to_dict() if "to_dict" in dir(x) else x) for x in data] parameters[key] = data # Construct HTML, load Vue Components source files only if the openEO HTML tag is not yet defined diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 0b9d024ca..5d53b4c4f 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -33,6 +33,7 @@ from openeo.rest.datacube import DataCube from openeo.rest.imagecollectionclient import ImageCollectionClient from openeo.rest.mlmodel import MlModel +from openeo.rest.userfile import UserFile from openeo.rest.job import BatchJob, RESTJob from openeo.rest.rest_capabilities import RESTCapabilities from openeo.rest.service import Service @@ -1092,20 +1093,20 @@ def list_files(self): """ Lists all files that the logged in user uploaded. - :return: file_list: List of the user uploaded files. + :return: file_list: List of the user-uploaded files. """ files = self.get('/files', expected_status=200).json()['files'] + files = [UserFile(file['path'], self, file) for file in files] return VisualList("data-table", data=files, parameters={'columns': 'files'}) - def create_file(self, path): + def get_file(self, path) -> UserFile: """ - Creates virtual file + Gets a (virtual) file for the user workspace - :return: file object. + :return: UserFile object. """ - # No endpoint just returns a file object. - raise NotImplementedError() + return UserFile(path, self) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py new file mode 100644 index 000000000..bda53952f --- /dev/null +++ b/openeo/rest/userfile.py @@ -0,0 +1,58 @@ +import typing +from typing import Any, Dict, List, Union +from pathlib import Path + +if typing.TYPE_CHECKING: + # Imports for type checking only (circular import issue at runtime). + from openeo.rest.connection import Connection + + +class UserFile: + """Represents a file in the user-workspace of openeo.""" + + def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = {}): + self.path = path + self.metadata = metadata + self.connection = connection + + def __repr__(self): + return '<{c} file={i!r}>'.format(c=self.__class__.__name__, i=self.path) + + def _get_endpoint(self) -> str: + return "/files/{}".format(self.path) + + def get_metadata(self, key) -> Any: + """ Get metadata about the file, e.g. file size (key: 'size') or modification date (key: 'modified').""" + if key in self.metadata: + return self.metadata[key] + else: + return None + + def download_file(self, target: Union[Path, str]) -> Path: + """ Downloads a user-uploaded file.""" + # GET /files/{path} + response = self.connection.get(self._get_endpoint(), expected_status=200, stream=True) + + path = Path(target) + with path.open(mode="wb") as f: + for chunk in response.iter_content(chunk_size=None): + f.write(chunk) + + return path + + + def upload_file(self, source: Union[Path, str]): + # PUT /files/{path} + """ Uploaded (or replaces) a user-uploaded file.""" + path = Path(source) + with path.open(mode="rb") as f: + self.connection.put(self._get_endpoint(), expected_status=200, data=f) + + def delete_file(self): + """ Delete a user-uploaded file.""" + # DELETE /files/{path} + self.connection.delete(self._get_endpoint(), expected_status=204) + + def to_dict(self) -> Dict[str, Any]: + """ Returns the provided metadata as dict.""" + return self.metadata if "path" in self.metadata else {"path": self.path} \ No newline at end of file From d773567e0dc20cb902306f9969f83da66a5210cb Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Fri, 17 Feb 2023 16:20:26 +0100 Subject: [PATCH 02/13] Add CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c49eb158..a89d3ac00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The openeo Python client library can now also be installed with conda (conda-forge channel) ([#176](https://github.com/Open-EO/openeo-python-client/issues/176)) - Allow using a custom `requests.Session` in `openeo.rest.auth.oidc` logic +- Full support for user-uploaded files (`/files` endpoints) [#377](https://github.com/Open-EO/openeo-python-client/issues/377) ### Changed - Less verbose log printing on failed batch job [#332](https://github.com/Open-EO/openeo-python-client/issues/332) - Improve (UTC) timezone handling in `openeo.util.Rfc3339` and add `rfc3339.today()`/`rfc3339.utcnow()`. +- `list_files()` returns a list of `UserFile` objects instead of a list of dictionaries - use `to_dict` on the `UserFile` object to get the dictionary. ### Removed From c7154b0c46e69444f8e88616b2f50ab22d441365 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Fri, 17 Feb 2023 23:35:58 +0100 Subject: [PATCH 03/13] Apply suggestions from code review Co-authored-by: Stefaan Lippens --- openeo/rest/connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 5d53b4c4f..dc6fffc4b 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1097,7 +1097,7 @@ def list_files(self): """ files = self.get('/files', expected_status=200).json()['files'] - files = [UserFile(file['path'], self, file) for file in files] + files = [UserFile(file['path'], connection=self, metadata=file) for file in files] return VisualList("data-table", data=files, parameters={'columns': 'files'}) def get_file(self, path) -> UserFile: @@ -1106,7 +1106,7 @@ def get_file(self, path) -> UserFile: :return: UserFile object. """ - return UserFile(path, self) + return UserFile(path, connection=self) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ From c040bab46207a14d20178ade82cfc4759e89da18 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Fri, 17 Feb 2023 23:52:59 +0100 Subject: [PATCH 04/13] Apply suggestions from code review --- openeo/rest/connection.py | 12 +++++++++++- openeo/rest/userfile.py | 39 ++++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index dc6fffc4b..6615bbd9f 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -5,6 +5,7 @@ import json import logging import shlex +import os import sys import warnings from collections import OrderedDict @@ -1100,7 +1101,7 @@ def list_files(self): files = [UserFile(file['path'], connection=self, metadata=file) for file in files] return VisualList("data-table", data=files, parameters={'columns': 'files'}) - def get_file(self, path) -> UserFile: + def get_file(self, path: str) -> UserFile: """ Gets a (virtual) file for the user workspace @@ -1108,6 +1109,15 @@ def get_file(self, path) -> UserFile: """ return UserFile(path, connection=self) + def upload_file(self, source: Union[Path, str]) -> UserFile: + """ + Uploads a file to the user workspace and stores it with the filename of the source given. + If a file with the name exists on the user workspace it will be replaced. + + :return: UserFile object. + """ + return self.get_file(os.path.basename(source)).upload(source) + def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ Prepare a json payload with a process graph to submit to /result, /services, /jobs, ... diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index bda53952f..e116bf2dc 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -1,6 +1,8 @@ import typing -from typing import Any, Dict, List, Union +from typing import Any, Dict, Union +import os from pathlib import Path +from openeo.util import ensure_dir if typing.TYPE_CHECKING: # Imports for type checking only (circular import issue at runtime). @@ -10,9 +12,9 @@ class UserFile: """Represents a file in the user-workspace of openeo.""" - def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = {}): + def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = None): self.path = path - self.metadata = metadata + self.metadata = metadata or {"path": path} self.connection = connection def __repr__(self): @@ -21,38 +23,41 @@ def __repr__(self): def _get_endpoint(self) -> str: return "/files/{}".format(self.path) - def get_metadata(self, key) -> Any: - """ Get metadata about the file, e.g. file size (key: 'size') or modification date (key: 'modified').""" - if key in self.metadata: - return self.metadata[key] - else: - return None + def download(self, target: Union[Path, str] = None) -> Path: + """ + Downloads a user-uploaded file to the given location. - def download_file(self, target: Union[Path, str]) -> Path: - """ Downloads a user-uploaded file.""" + :param target: download target path. Can be an existing folder + (in which case the file name advertised by backend will be used) + or full file name. By default, the working directory will be used. + """ # GET /files/{path} response = self.connection.get(self._get_endpoint(), expected_status=200, stream=True) - path = Path(target) - with path.open(mode="wb") as f: + target = Path(target or Path.cwd()) + if target.is_dir(): + target = target / os.path.basename(self.path) + ensure_dir(target.parent) + + with target.open(mode="wb") as f: for chunk in response.iter_content(chunk_size=None): f.write(chunk) - return path + return target - def upload_file(self, source: Union[Path, str]): + def upload(self, source: Union[Path, str]): # PUT /files/{path} """ Uploaded (or replaces) a user-uploaded file.""" path = Path(source) with path.open(mode="rb") as f: self.connection.put(self._get_endpoint(), expected_status=200, data=f) - def delete_file(self): + def delete(self): """ Delete a user-uploaded file.""" # DELETE /files/{path} self.connection.delete(self._get_endpoint(), expected_status=204) def to_dict(self) -> Dict[str, Any]: """ Returns the provided metadata as dict.""" - return self.metadata if "path" in self.metadata else {"path": self.path} \ No newline at end of file + return self.metadata \ No newline at end of file From a7f52b5c4e9413045383859138fc9711f44604b8 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Mon, 6 Mar 2023 12:25:01 +0100 Subject: [PATCH 05/13] Apply suggestions from code review Co-authored-by: Stefaan Lippens --- openeo/internal/jupyter.py | 2 +- openeo/rest/connection.py | 5 ++--- openeo/rest/userfile.py | 7 +++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/openeo/internal/jupyter.py b/openeo/internal/jupyter.py index ac98588cf..5a10372af 100644 --- a/openeo/internal/jupyter.py +++ b/openeo/internal/jupyter.py @@ -103,7 +103,7 @@ def render_component(component: str, data = None, parameters: dict = None): key = COMPONENT_MAP.get(component, component) if data is not None: if isinstance(data, list): - data = [(x.to_dict() if "to_dict" in dir(x) else x) for x in data] + data = [(x.to_dict() if hasattr(x, "to_dict") else x) for x in data] parameters[key] = data # Construct HTML, load Vue Components source files only if the openEO HTML tag is not yet defined diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 6615bbd9f..cd39d3f19 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -5,7 +5,6 @@ import json import logging import shlex -import os import sys import warnings from collections import OrderedDict @@ -1103,7 +1102,7 @@ def list_files(self): def get_file(self, path: str) -> UserFile: """ - Gets a (virtual) file for the user workspace + Gets a handle to a file in the user workspace on the back-end. :return: UserFile object. """ @@ -1116,7 +1115,7 @@ def upload_file(self, source: Union[Path, str]) -> UserFile: :return: UserFile object. """ - return self.get_file(os.path.basename(source)).upload(source) + return self.get_file(Path(source).name).upload(source) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index e116bf2dc..43f0480ac 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -1,6 +1,5 @@ import typing from typing import Any, Dict, Union -import os from pathlib import Path from openeo.util import ensure_dir @@ -13,7 +12,7 @@ class UserFile: """Represents a file in the user-workspace of openeo.""" def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = None): - self.path = path + self.path = Path(path) self.metadata = metadata or {"path": path} self.connection = connection @@ -21,7 +20,7 @@ def __repr__(self): return '<{c} file={i!r}>'.format(c=self.__class__.__name__, i=self.path) def _get_endpoint(self) -> str: - return "/files/{}".format(self.path) + return f"/files/{self.path!s}" def download(self, target: Union[Path, str] = None) -> Path: """ @@ -36,7 +35,7 @@ def download(self, target: Union[Path, str] = None) -> Path: target = Path(target or Path.cwd()) if target.is_dir(): - target = target / os.path.basename(self.path) + target = target / self.path.name ensure_dir(target.parent) with target.open(mode="wb") as f: From 3766dc9bb79100e9385eda879284f182f4a3649e Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Mon, 6 Mar 2023 12:57:36 +0100 Subject: [PATCH 06/13] Improvements --- openeo/rest/connection.py | 14 ++++++++++---- openeo/rest/userfile.py | 15 ++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index cd39d3f19..f350279fd 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1104,18 +1104,24 @@ def get_file(self, path: str) -> UserFile: """ Gets a handle to a file in the user workspace on the back-end. + :param path: The path on the user workspace. :return: UserFile object. """ return UserFile(path, connection=self) - def upload_file(self, source: Union[Path, str]) -> UserFile: + def upload_file(self, source: Union[Path, str], target: str = None) -> UserFile: """ - Uploads a file to the user workspace and stores it with the filename of the source given. - If a file with the name exists on the user workspace it will be replaced. + Uploads a file to the given target location in the user workspace. + If a file at the target path exists in the user workspace it will be replaced. + + :param source: A path to a file on the local file system to upload. + :param target: The desired path on the user workspace. If not set, defaults to the filename (without path) of the local file. :return: UserFile object. """ - return self.get_file(Path(source).name).upload(source) + if target is None: + target = Path(source).name + return self.get_file(target).upload(source) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index 43f0480ac..215e6c3c8 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -43,11 +43,16 @@ def download(self, target: Union[Path, str] = None) -> Path: f.write(chunk) return target + + def upload(self, source: Union[Path, str], target = None): + """ + Uploads a file to the given target location in the user workspace. + If the file exists in the user workspace it will be replaced. - def upload(self, source: Union[Path, str]): + :param source: A path to a file on the local file system to upload. + """ # PUT /files/{path} - """ Uploaded (or replaces) a user-uploaded file.""" path = Path(source) with path.open(mode="rb") as f: self.connection.put(self._get_endpoint(), expected_status=200, data=f) @@ -58,5 +63,9 @@ def delete(self): self.connection.delete(self._get_endpoint(), expected_status=204) def to_dict(self) -> Dict[str, Any]: - """ Returns the provided metadata as dict.""" + """ + Returns the provided metadata as dict. + + This is used in internal/jupyter.py to detect and get the original metadata. + """ return self.metadata \ No newline at end of file From 37890a9a0992e227767cb17ac6012310841f2c55 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Mon, 6 Mar 2023 14:48:36 +0100 Subject: [PATCH 07/13] clean-up param, fix code style, use PurePosixPath --- openeo/rest/connection.py | 4 ++-- openeo/rest/userfile.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index f350279fd..65c67eff2 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -8,7 +8,7 @@ import sys import warnings from collections import OrderedDict -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Dict, List, Tuple, Union, Callable, Optional, Any, Iterator from urllib.parse import urljoin @@ -1120,7 +1120,7 @@ def upload_file(self, source: Union[Path, str], target: str = None) -> UserFile: :return: UserFile object. """ if target is None: - target = Path(source).name + target = PurePosixPath(source).name return self.get_file(target).upload(source) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index 215e6c3c8..7b764049e 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -1,6 +1,6 @@ import typing from typing import Any, Dict, Union -from pathlib import Path +from pathlib import Path, PurePosixPath from openeo.util import ensure_dir if typing.TYPE_CHECKING: @@ -12,7 +12,7 @@ class UserFile: """Represents a file in the user-workspace of openeo.""" def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = None): - self.path = Path(path) + self.path = PurePosixPath(path) self.metadata = metadata or {"path": path} self.connection = connection @@ -26,25 +26,25 @@ def download(self, target: Union[Path, str] = None) -> Path: """ Downloads a user-uploaded file to the given location. - :param target: download target path. Can be an existing folder - (in which case the file name advertised by backend will be used) + :param target: download target path. Can be an existing folder + (in which case the file name advertised by backend will be used) or full file name. By default, the working directory will be used. """ # GET /files/{path} response = self.connection.get(self._get_endpoint(), expected_status=200, stream=True) - target = Path(target or Path.cwd()) + target = Path(target or Path.cwd()) if target.is_dir(): target = target / self.path.name ensure_dir(target.parent) - + with target.open(mode="wb") as f: for chunk in response.iter_content(chunk_size=None): f.write(chunk) return target - - def upload(self, source: Union[Path, str], target = None): + + def upload(self, source: Union[Path, str]): """ Uploads a file to the given target location in the user workspace. @@ -68,4 +68,4 @@ def to_dict(self) -> Dict[str, Any]: This is used in internal/jupyter.py to detect and get the original metadata. """ - return self.metadata \ No newline at end of file + return self.metadata From 7ae3f0ec244f0a2e60345260f7892e689e28e051 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 15:38:27 +0100 Subject: [PATCH 08/13] PR #378: add some tests and additional finetuning --- openeo/rest/connection.py | 16 ++-- openeo/rest/userfile.py | 9 +- tests/rest/test_userfile.py | 168 ++++++++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 tests/rest/test_userfile.py diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 65c67eff2..f966b1c1f 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1089,9 +1089,9 @@ def job_logs(self, job_id, offset) -> list: """Get batch job logs.""" return BatchJob(job_id=job_id, connection=self).logs(offset=offset) - def list_files(self): + def list_files(self) -> List[UserFile]: """ - Lists all files that the logged in user uploaded. + Lists all files that the logged-in user uploaded. :return: file_list: List of the user-uploaded files. """ @@ -1100,16 +1100,20 @@ def list_files(self): files = [UserFile(file['path'], connection=self, metadata=file) for file in files] return VisualList("data-table", data=files, parameters={'columns': 'files'}) - def get_file(self, path: str) -> UserFile: + def get_file(self, path: Union[str, PurePosixPath]) -> UserFile: """ Gets a handle to a file in the user workspace on the back-end. - :param path: The path on the user workspace. + :param path: The path on the user workspace. :return: UserFile object. """ return UserFile(path, connection=self) - def upload_file(self, source: Union[Path, str], target: str = None) -> UserFile: + def upload_file( + self, + source: Union[Path, str], + target: Optional[Union[str, PurePosixPath]] = None, + ) -> UserFile: """ Uploads a file to the given target location in the user workspace. @@ -1120,7 +1124,7 @@ def upload_file(self, source: Union[Path, str], target: str = None) -> UserFile: :return: UserFile object. """ if target is None: - target = PurePosixPath(source).name + target = Path(source).name return self.get_file(target).upload(source) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index 7b764049e..957b9336e 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -11,7 +11,12 @@ class UserFile: """Represents a file in the user-workspace of openeo.""" - def __init__(self, path: str, connection: 'Connection', metadata: Dict[str, Any] = None): + def __init__( + self, + path: Union[str, PurePosixPath], + connection: "Connection", + metadata: Dict[str, Any] = None, + ): self.path = PurePosixPath(path) self.metadata = metadata or {"path": path} self.connection = connection @@ -53,8 +58,10 @@ def upload(self, source: Union[Path, str]): :param source: A path to a file on the local file system to upload. """ # PUT /files/{path} + # TODO: support other non-path sources too: bytes, open file, url, ... path = Path(source) with path.open(mode="rb") as f: + # TODO: `PUT /files/{path} results in new file metadata, so should we return a new UserFile here? self.connection.put(self._get_endpoint(), expected_status=200, data=f) def delete(self): diff --git a/tests/rest/test_userfile.py b/tests/rest/test_userfile.py new file mode 100644 index 000000000..b45f3b99d --- /dev/null +++ b/tests/rest/test_userfile.py @@ -0,0 +1,168 @@ +from openeo.rest import OpenEoApiError +from openeo.rest.userfile import UserFile +from pathlib import Path, PurePosixPath +import pytest +import openeo +from openeo.util import rfc3339 + +API_URL = "https://oeo.test" + + +@pytest.fixture +def con100(requests_mock) -> openeo.Connection: + requests_mock.get(API_URL + "/", json={"api_version": "1.0.0"}) + con = openeo.connect(API_URL) + return con + + +class TestUserFile: + def test_simple(self, con100): + f = UserFile("foo.txt", connection=con100) + assert f.path == PurePosixPath("foo.txt") + + def test_connection_get_file(self, con100): + f = con100.get_file("foo.txt") + assert f.path == PurePosixPath("foo.txt") + + def test_connection_list_files(self, con100, requests_mock): + requests_mock.get( + API_URL + "/files", + json={ + "files": [ + { + "path": "foo.txt", + "size": 182, + "modified": "2015-10-20T17:22:10Z", + }, + { + "path": "data/foo.tiff", + "size": 183142, + "modified": "2017-01-01T09:36:18Z", + }, + ], + }, + ) + files = con100.list_files() + assert len(files) + assert files[0].path == PurePosixPath("foo.txt") + assert files[0].metadata == { + "path": "foo.txt", + "size": 182, + "modified": "2015-10-20T17:22:10Z", + } + assert files[1].path == PurePosixPath("data/foo.tiff") + assert files[1].metadata == { + "path": "data/foo.tiff", + "size": 183142, + "modified": "2017-01-01T09:36:18Z", + } + + def test_upload(self, con100, tmp_path, requests_mock): + source = tmp_path / "to-upload.txt" + source.write_text("hello world\n") + f = UserFile("foo.txt", connection=con100) + + def put_files(request, context): + import io + + if isinstance(request.text, io.BufferedReader): + body = request.text.read() + else: + body = request.text + assert body == b"hello world\n" + context.status_code = 200 + return { + "path": "foo.txt", + "size": len(body), + "modified": "2018-01-03T10:55:29Z", + } + + upload_mock = requests_mock.put(API_URL + "/files/foo.txt", json=put_files) + + # TODO: check response? + f.upload(source) + + assert upload_mock.call_count == 1 + + @pytest.mark.parametrize("status_code", [404, 500]) + def test_upload_fail(self, tmp_path, con100, requests_mock, status_code): + source = tmp_path / "to-upload.txt" + source.write_text("hello world\n") + f = UserFile("foo.txt", connection=con100) + + upload_mock = requests_mock.put( + API_URL + "/files/foo.txt", status_code=status_code + ) + + with pytest.raises(OpenEoApiError, match=rf"\[{status_code}\]"): + f.upload(source) + + assert upload_mock.call_count == 1 + + @pytest.mark.parametrize( + ["target", "expected_target"], + [ + (None, "to-upload.txt"), + ("foo.txt", "foo.txt"), + ], + ) + def test_connection_upload( + self, con100, tmp_path, requests_mock, target, expected_target + ): + source = tmp_path / "to-upload.txt" + source.write_text("hello world\n") + + def put_files(request, context): + import io + + if isinstance(request.text, io.BufferedReader): + body = request.text.read() + else: + body = request.text + assert body == b"hello world\n" + context.status_code = 200 + return { + "path": request.path, + "size": len(body), + "modified": "2018-01-03T10:55:29Z", + } + + upload_mock = requests_mock.put( + API_URL + f"/files/{expected_target}", json=put_files + ) + + f = con100.upload_file(source, target=target) + assert upload_mock.call_count == 1 + + # TODO + # assert f.path == PurePosixPath(expected_target) + # assert f.metadata == {} + + @pytest.mark.parametrize( + ["target", "expected"], + [ + (None, "foo.txt"), + ("data", "data/foo.txt"), + ("data/bar.txt", "data/bar.txt"), + ], + ) + def test_download( + self, con100, tmp_path, requests_mock, target, expected, monkeypatch + ): + monkeypatch.chdir(tmp_path) + (tmp_path / "data").mkdir(parents=True, exist_ok=True) + + download_mock = requests_mock.get( + API_URL + "/files/foo.txt", content=b"hello world\n" + ) + + if target is not None: + target = tmp_path / target + + f = UserFile("foo.txt", connection=con100) + f.download(target) + + expected = tmp_path / expected + assert expected.exists() + assert expected.read_bytes() == b"hello world\n" + assert download_mock.call_count == 1 From 20199abd8c0a47314693a57257614db2808caea5 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 16:50:50 +0100 Subject: [PATCH 09/13] PR #378 move upload logic to Connection Because upload creates a new UserFile it is more untuitive to have main logic in Connection. --- openeo/rest/connection.py | 18 ++++++++++----- openeo/rest/userfile.py | 31 +++++++++++++++++--------- tests/rest/test_userfile.py | 44 ++++++++++++++++++++++++++++++++----- 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index f966b1c1f..35d44db61 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1097,17 +1097,19 @@ def list_files(self) -> List[UserFile]: """ files = self.get('/files', expected_status=200).json()['files'] - files = [UserFile(file['path'], connection=self, metadata=file) for file in files] + files = [UserFile.from_metadata(metadata=f, connection=self) for f in files] return VisualList("data-table", data=files, parameters={'columns': 'files'}) - def get_file(self, path: Union[str, PurePosixPath]) -> UserFile: + def get_file( + self, path: Union[str, PurePosixPath], metadata: Optional[dict] = None + ) -> UserFile: """ Gets a handle to a file in the user workspace on the back-end. :param path: The path on the user workspace. :return: UserFile object. """ - return UserFile(path, connection=self) + return UserFile(path=path, connection=self, metadata=metadata) def upload_file( self, @@ -1123,9 +1125,13 @@ def upload_file( :param target: The desired path on the user workspace. If not set, defaults to the filename (without path) of the local file. :return: UserFile object. """ - if target is None: - target = Path(source).name - return self.get_file(target).upload(source) + source = Path(source) + target = target or source.name + # TODO: support other non-path sources too: bytes, open file, url, ... + with source.open("rb") as f: + resp = self.put(f"/files/{target!s}", expected_status=200, data=f) + metadata = resp.json() + return UserFile.from_metadata(metadata=metadata, connection=self) def _build_request_with_process_graph(self, process_graph: Union[dict, Any], **kwargs) -> dict: """ diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index 957b9336e..bf961a7bf 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -1,5 +1,5 @@ import typing -from typing import Any, Dict, Union +from typing import Any, Dict, Union, Optional from pathlib import Path, PurePosixPath from openeo.util import ensure_dir @@ -13,14 +13,29 @@ class UserFile: def __init__( self, - path: Union[str, PurePosixPath], + path: Union[str, PurePosixPath, None], + *, connection: "Connection", - metadata: Dict[str, Any] = None, + metadata: Optional[dict] = None, ): + if path: + pass + elif metadata and metadata.get("path"): + path = metadata.get("path") + else: + raise ValueError( + "File path should be specified through `path` or `metadata` argument." + ) + self.path = PurePosixPath(path) self.metadata = metadata or {"path": path} self.connection = connection + @classmethod + def from_metadata(cls, metadata: dict, connection: "Connection") -> "UserFile": + """Build :py:class:`UserFile` from workspace file metadata dictionary.""" + return cls(path=None, connection=connection, metadata=metadata) + def __repr__(self): return '<{c} file={i!r}>'.format(c=self.__class__.__name__, i=self.path) @@ -49,20 +64,16 @@ def download(self, target: Union[Path, str] = None) -> Path: return target - def upload(self, source: Union[Path, str]): + def upload(self, source: Union[Path, str]) -> "UserFile": """ Uploads a file to the given target location in the user workspace. If the file exists in the user workspace it will be replaced. - :param source: A path to a file on the local file system to upload. + :param source: A path to a file on the local file system to upload. """ # PUT /files/{path} - # TODO: support other non-path sources too: bytes, open file, url, ... - path = Path(source) - with path.open(mode="rb") as f: - # TODO: `PUT /files/{path} results in new file metadata, so should we return a new UserFile here? - self.connection.put(self._get_endpoint(), expected_status=200, data=f) + return self.connection.upload_file(source, target=self.path) def delete(self): """ Delete a user-uploaded file.""" diff --git a/tests/rest/test_userfile.py b/tests/rest/test_userfile.py index b45f3b99d..e7c1d3b3a 100644 --- a/tests/rest/test_userfile.py +++ b/tests/rest/test_userfile.py @@ -1,3 +1,5 @@ +import re + from openeo.rest import OpenEoApiError from openeo.rest.userfile import UserFile from pathlib import Path, PurePosixPath @@ -19,10 +21,17 @@ class TestUserFile: def test_simple(self, con100): f = UserFile("foo.txt", connection=con100) assert f.path == PurePosixPath("foo.txt") + assert f.metadata == {"path": "foo.txt"} + + def test_from_metadata(self, con100): + f = UserFile.from_metadata(metadata={"path": "foo.txt"}, connection=con100) + assert f.path == PurePosixPath("foo.txt") + assert f.metadata == {"path": "foo.txt"} def test_connection_get_file(self, con100): f = con100.get_file("foo.txt") assert f.path == PurePosixPath("foo.txt") + assert f.metadata == {"path": "foo.txt"} def test_connection_list_files(self, con100, requests_mock): requests_mock.get( @@ -79,8 +88,13 @@ def put_files(request, context): upload_mock = requests_mock.put(API_URL + "/files/foo.txt", json=put_files) - # TODO: check response? - f.upload(source) + f2 = f.upload(source) + assert f2.path == PurePosixPath("foo.txt") + assert f2.metadata == { + "path": "foo.txt", + "size": 12, + "modified": "2018-01-03T10:55:29Z", + } assert upload_mock.call_count == 1 @@ -104,6 +118,7 @@ def test_upload_fail(self, tmp_path, con100, requests_mock, status_code): [ (None, "to-upload.txt"), ("foo.txt", "foo.txt"), + ("data/foo.txt", "data/foo.txt"), ], ) def test_connection_upload( @@ -122,7 +137,7 @@ def put_files(request, context): assert body == b"hello world\n" context.status_code = 200 return { - "path": request.path, + "path": re.match("/files/(.*)", request.path).group(1), "size": len(body), "modified": "2018-01-03T10:55:29Z", } @@ -134,9 +149,26 @@ def put_files(request, context): f = con100.upload_file(source, target=target) assert upload_mock.call_count == 1 - # TODO - # assert f.path == PurePosixPath(expected_target) - # assert f.metadata == {} + assert f.path == PurePosixPath(expected_target) + assert f.metadata == { + "path": expected_target, + "size": 12, + "modified": "2018-01-03T10:55:29Z", + } + + @pytest.mark.parametrize("status_code", [404, 500]) + def test_connection_upload_fail(self, con100, tmp_path, requests_mock, status_code): + source = tmp_path / "foo.txt" + source.write_text("hello world\n") + + upload_mock = requests_mock.put( + API_URL + "/files/foo.txt", status_code=status_code + ) + + with pytest.raises(OpenEoApiError, match=rf"\[{status_code}\]"): + con100.upload_file(source=source) + + assert upload_mock.call_count == 1 @pytest.mark.parametrize( ["target", "expected"], From b2581d78b7a3e5dc388459772f610d4189ab59a5 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 17:09:12 +0100 Subject: [PATCH 10/13] PR #378 Add UserFile to API docs Finetune/fix UserFile docs as well --- docs/api.rst | 7 +++++++ openeo/rest/connection.py | 14 ++++++-------- openeo/rest/userfile.py | 34 ++++++++++++++++++---------------- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 947f79595..e9303eeea 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -86,6 +86,13 @@ openeo.rest.udp :members: RESTUserDefinedProcess +openeo.rest.userfile +---------------------- + +.. automodule:: openeo.rest.userfile + :members: + + openeo.udf ------------- diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 35d44db61..660433662 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1091,11 +1091,10 @@ def job_logs(self, job_id, offset) -> list: def list_files(self) -> List[UserFile]: """ - Lists all files that the logged-in user uploaded. + Lists all user-uploaded files in the user workspace on the back-end. - :return: file_list: List of the user-uploaded files. + :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'}) @@ -1104,10 +1103,9 @@ def get_file( self, path: Union[str, PurePosixPath], metadata: Optional[dict] = None ) -> UserFile: """ - Gets a handle to a file in the user workspace on the back-end. + Gets a handle to a user-uploaded file in the user workspace on the back-end. :param path: The path on the user workspace. - :return: UserFile object. """ return UserFile(path=path, connection=self, metadata=metadata) @@ -1117,13 +1115,13 @@ def upload_file( target: Optional[Union[str, PurePosixPath]] = None, ) -> UserFile: """ - Uploads a file to the given target location in the user workspace. + Uploads a file to the given target location in the user workspace on the back-end. If a file at the target path exists in the user workspace it will be replaced. :param source: A path to a file on the local file system to upload. - :param target: The desired path on the user workspace. If not set, defaults to the filename (without path) of the local file. - :return: UserFile object. + :param target: The desired path on the user workspace. + If not set, defaults to the filename (without path) of the local file. """ source = Path(source) target = target or source.name diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index bf961a7bf..271cbee4f 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -9,7 +9,9 @@ class UserFile: - """Represents a file in the user-workspace of openeo.""" + """ + Handle to a (user-uploaded) file in the user workspace on a openEO back-end. + """ def __init__( self, @@ -33,25 +35,27 @@ def __init__( @classmethod def from_metadata(cls, metadata: dict, connection: "Connection") -> "UserFile": - """Build :py:class:`UserFile` from workspace file metadata dictionary.""" + """Build :py:class:`UserFile` from a workspace file metadata dictionary.""" return cls(path=None, connection=connection, metadata=metadata) def __repr__(self): - return '<{c} file={i!r}>'.format(c=self.__class__.__name__, i=self.path) + return "<{c} file={i!r}>".format(c=self.__class__.__name__, i=self.path) def _get_endpoint(self) -> str: return f"/files/{self.path!s}" def download(self, target: Union[Path, str] = None) -> Path: """ - Downloads a user-uploaded file to the given location. + Downloads a user-uploaded file from the user workspace on the back-end + locally to the given location. - :param target: download target path. Can be an existing folder + :param target: local download target path. Can be an existing folder (in which case the file name advertised by backend will be used) or full file name. By default, the working directory will be used. """ - # GET /files/{path} - response = self.connection.get(self._get_endpoint(), expected_status=200, stream=True) + response = self.connection.get( + self._get_endpoint(), expected_status=200, stream=True + ) target = Path(target or Path.cwd()) if target.is_dir(): @@ -66,24 +70,22 @@ def download(self, target: Union[Path, str] = None) -> Path: def upload(self, source: Union[Path, str]) -> "UserFile": """ - Uploads a file to the given target location in the user workspace. + Uploads a local file to the given target location in the user workspace + and returns new :py:class:`UserFile` of newly uploaded file. If the file exists in the user workspace it will be replaced. :param source: A path to a file on the local file system to upload. + :return: new :py:class:`UserFile` instance of the newly uploaded file """ - # PUT /files/{path} return self.connection.upload_file(source, target=self.path) def delete(self): - """ Delete a user-uploaded file.""" - # DELETE /files/{path} + """Delete the user-uploaded file from the user workspace on the back-end.""" self.connection.delete(self._get_endpoint(), expected_status=204) def to_dict(self) -> Dict[str, Any]: - """ - Returns the provided metadata as dict. - - This is used in internal/jupyter.py to detect and get the original metadata. - """ + """Returns the provided metadata as dict.""" + # This is used in internal/jupyter.py to detect and get the original metadata. + # TODO: make this more explicit with an internal API? return self.metadata From 7e1facdba8943156752fd8443aecac5cd253ee47 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 17:30:25 +0100 Subject: [PATCH 11/13] PR #378 finetune changelog --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a89d3ac00..55e6ec593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,13 +12,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The openeo Python client library can now also be installed with conda (conda-forge channel) ([#176](https://github.com/Open-EO/openeo-python-client/issues/176)) - Allow using a custom `requests.Session` in `openeo.rest.auth.oidc` logic -- Full support for user-uploaded files (`/files` endpoints) [#377](https://github.com/Open-EO/openeo-python-client/issues/377) +- Full support for user-uploaded files (`/files` endpoints) + ([#377](https://github.com/Open-EO/openeo-python-client/issues/377)) ### Changed - Less verbose log printing on failed batch job [#332](https://github.com/Open-EO/openeo-python-client/issues/332) - Improve (UTC) timezone handling in `openeo.util.Rfc3339` and add `rfc3339.today()`/`rfc3339.utcnow()`. -- `list_files()` returns a list of `UserFile` objects instead of a list of dictionaries - use `to_dict` on the `UserFile` object to get the dictionary. +- `Connection.list_files()` returns a list of `UserFile` objects instead of a list of metadata dictionaries. + Use `UserFile.metadata` to get the original dictionary. + ([#377](https://github.com/Open-EO/openeo-python-client/issues/377)) ### Removed From ee58d5f437ee0da44fd9fe3fe62c6592b91b64d5 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 17:40:27 +0100 Subject: [PATCH 12/13] fixup! PR #378 Add UserFile to API docs --- openeo/rest/connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index 660433662..06f3d7fc8 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -1120,8 +1120,8 @@ def upload_file( If a file at the target path exists in the user workspace it will be replaced. :param source: A path to a file on the local file system to upload. - :param target: The desired path on the user workspace. - If not set, defaults to the filename (without path) of the local file. + :param target: The desired path (which can contain a folder structure if desired) on the user workspace. + If not set: defaults to the original filename (without any folder structure) of the local file . """ source = Path(source) target = target or source.name From adf87d798215e908548f75cbefde425bdb38b069 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Mon, 6 Mar 2023 17:41:17 +0100 Subject: [PATCH 13/13] fixup! PR #378 Add UserFile to API docs --- openeo/rest/userfile.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openeo/rest/userfile.py b/openeo/rest/userfile.py index 271cbee4f..e1daca290 100644 --- a/openeo/rest/userfile.py +++ b/openeo/rest/userfile.py @@ -70,9 +70,14 @@ def download(self, target: Union[Path, str] = None) -> Path: def upload(self, source: Union[Path, str]) -> "UserFile": """ - Uploads a local file to the given target location in the user workspace + Uploads a local file to the path corresponding to this :py:class:`UserFile` in the user workspace and returns new :py:class:`UserFile` of newly uploaded file. + .. tip:: + Usually you'll just need + :py:meth:`Connection.upload_file() ` + instead of this :py:class:`UserFile` method. + If the file exists in the user workspace it will be replaced. :param source: A path to a file on the local file system to upload.