From 471f7413a70ac2648ce61ff019de5abf7a45e847 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Wed, 27 Mar 2024 11:15:30 -0700 Subject: [PATCH 1/9] add semver versioning example --- caveclient/chunkedgraph.py | 60 +++++++++++++++++++++++++++++++++----- caveclient/endpoints.py | 1 + setup.py | 8 ++--- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index a7f3680e..329e074c 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -2,14 +2,15 @@ import datetime import json import logging -from typing import Iterable, Tuple, Union +from typing import Iterable, Optional, Tuple, Union from urllib.parse import urlencode import networkx as nx import numpy as np -import pandas as pd import pytz +import pandas as pd + from .auth import AuthClient from .base import BaseEncoder, ClientBase, _api_endpoints, handle_response from .endpoints import ( @@ -182,6 +183,7 @@ def __init__( self._default_timestamp = timestamp self._table_name = table_name self._segmentation_info = None + self._semver = self._get_current_semver() @property def default_url_mapping(self): @@ -191,6 +193,48 @@ def default_url_mapping(self): def table_name(self): return self._table_name + # TODO refactor to base client + def _get_current_semver(self): + endpoint_mapping = self.default_url_mapping + url = self._endpoints["get_current_semver"].format_map(endpoint_mapping) + response = self.session.get(url) + if response.status_code == 404: # server doesn't have this endpoint yet + return None + else: + return response.json() + + @property + def major_version(self) -> Optional[int]: + if self._semver is None: + return None + else: + return int(self._semver.split(".")[0]) + + @property + def minor_version(self) -> Optional[int]: + if self._semver is None: + return None + else: + return int(self._semver.split(".")[1]) + + @property + def patch_version(self) -> Optional[int]: + if self._semver is None: + return None + else: + return int(self._semver.split(".")[2]) + + @property + def version(self) -> Optional[str]: + return self._semver + + @property + def max_version(self) -> str: + if self._semver is None: + return "2.15.0" # this is the last version that doesn't have the endpoints + else: + return self._semver + def _process_timestamp(self, timestamp): """Process timestamp with default logic""" if timestamp is None: @@ -785,11 +829,11 @@ def level2_chunk_graph(self, root_id, bounds=None) -> list: Root id of object bounds : np.array 3x2 bounding box (x,y,z) x (min,max) in chunked graph coordinates (use - `client.chunkedgraph.base_resolution` to view this default resolution for - your chunkedgraph client). Note that the result will include any level 2 - nodes which have chunk boundaries within some part of this bounding box, - meaning that the representative point for a given level 2 node could still - be slightly outside of these bounds. If None, returns all level 2 chunks + `client.chunkedgraph.base_resolution` to view this default resolution for + your chunkedgraph client). Note that the result will include any level 2 + nodes which have chunk boundaries within some part of this bounding box, + meaning that the representative point for a given level 2 node could still + be slightly outside of these bounds. If None, returns all level 2 chunks for the root ID. Returns @@ -818,7 +862,7 @@ def level2_chunk_graph(self, root_id, bounds=None) -> list: "your system administrator to update the chunkedgraph." ) raise ValueError(warning) - + r = handle_response(response) return r["edge_graph"] diff --git a/caveclient/endpoints.py b/caveclient/endpoints.py index 2bd158bb..d40f92e4 100644 --- a/caveclient/endpoints.py +++ b/caveclient/endpoints.py @@ -128,6 +128,7 @@ pcg_common = "{cg_server_address}/segmentation" chunkedgraph_endpoints_common = { "get_api_versions": pcg_common + "/api/versions", + "get_current_semver": pcg_common + "/api/current_semver", "info": pcg_common + "/table/{table_id}/info", } diff --git a/setup.py b/setup.py index f7ac297b..ade6f1d4 100644 --- a/setup.py +++ b/setup.py @@ -26,17 +26,17 @@ def find_version(*file_paths): # read the contents of README file this_directory = Path(__file__).parent -long_description = (this_directory / "README.rst").read_text() +long_description = (this_directory / "README.md").read_text() setup( version=find_version("caveclient", "__init__.py"), name="caveclient", - description="a service for interacting with the Connectome Annotation Versioning Engine", + description="A client for interacting with the Connectome Annotation Versioning Engine", long_description=long_description, - long_description_content_type="text/x-rst", + long_description_content_type="text/markdown", author="Forrest Collman, Casey Schneider-Mizell, Sven Dorkenwald", author_email="forrestc@alleninstute.org,caseys@alleninstitute.org,svenmd@princeton.edu,", - url="https://github.com/seung-lab/CAVEclient", + url="https://github.com/CAVEconnectome/CAVEclient", packages=find_packages(where="."), include_package_data=True, install_requires=required, From a6144e67287ab81bef470bf0f241677f88a14f44 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Wed, 27 Mar 2024 12:29:39 -0700 Subject: [PATCH 2/9] add a version of constraint checking --- caveclient/base.py | 140 ++++++++++++++++++++++++++++++++++++- caveclient/chunkedgraph.py | 9 ++- 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index aa4f78ef..ca2a2617 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -1,12 +1,18 @@ import datetime import json import logging +import operator import urllib import webbrowser +from functools import wraps +from typing import Callable import numpy as np -import pandas as pd import requests +from packaging.version import Version +from packaging.version import parse as parse_version + +import pandas as pd from .session_config import patch_session @@ -299,3 +305,135 @@ def __init__( @property def datastack_name(self): return self._datastack_name + + +def parametrized(dec): + """This decorator allows you to easily create decorators that take arguments""" + # REF: https://stackoverflow.com/questions/5929107/decorators-with-parameters + + @wraps(dec) + def layer(*args, **kwargs): + @wraps(dec) + def repl(f): + return dec(f, *args, **kwargs) + + return repl + + return layer + + +def _extract_constraint_info(constraint: str) -> tuple[str, Callable, Version]: + """ + Extracts the operator and version number from a version constraint. + + Parameters + ---------- + constraint + Version constraint described as a comparison operator followed by the version + number. For example, "<=1.0.0" would indicate that this method is only + compatible with server versions less than or equal to 1.0.0. + + Returns + ------- + : + The complement name of the constraint. + : + The complement operator of the constraint. + : + The version object of the constraint. + + """ + if "<=" == constraint[:2]: + complement_name = ">" + complement_operator = operator.gt + constraint = constraint[2:] + elif ">=" == constraint[:2]: + complement_name = "<" + complement_operator = operator.lt + constraint = constraint[2:] + elif "==" == constraint[:2]: + complement_name = "!=" + complement_operator = operator.ne + constraint = constraint[2:] + elif "!=" == constraint[:2]: + complement_name = "==" + complement_operator = operator.eq + constraint = constraint[2:] + elif "<" == constraint[0]: + complement_name = ">=" + complement_operator = operator.ge + constraint = constraint[1:] + elif ">" == constraint[0]: + complement_name = "<=" + complement_operator = operator.le + constraint = constraint[1:] + else: + raise ValueError(f"Constraint {constraint} not recognized.") + constraint = parse_version(constraint) + return complement_name, complement_operator, constraint + + +@parametrized +def check_version_compatibility( + method: Callable, method_constraint: str = None, kwarg_use_constraints: dict = None +) -> Callable: + """ + This decorator is used to check the compatibility features in the client and + server versions. If the server version is not compatible with the constraint, an + error will be raised. + + Parameters + ---------- + method + Method to be decorated. + method_constraint + Version constraint for the method, described as a comparison operator + followed by the version number. For example, "<=1.0.0" would indicate that this + method is only compatible with server versions less than or equal to 1.0.0. + kwarg_use_constraints + Dictionary with some number of the method's keyword arguments as keys and + version constraints as values. Version constraints are described as a + comparison operator followed by the version number. For example, "<=1.0.0" + would indicate that the keyword argument is only compatible with server versions + less than or equal to 1.0.0. An error will be raised only if the user both + provides the keyword argument (even if passing in the default value!) and the + server version is not compatible with the constraint. + """ + + @wraps(method) + def wrapper(*args, **kwargs): + self = args[0] + server_version = parse_version(self.version) + + if method_constraint is not None: + complement_name, complement_operator, constraint = _extract_constraint_info( + method_constraint + ) + + if complement_operator(server_version, constraint): + msg = ( + f"Method {method.__name__} is not permitted " + f"for server version {complement_name}{constraint}, your server " + f"version is {server_version}. Contact your system " + "administrator to update the server version." + ) + raise ValueError(msg) + + for kwarg, value in kwarg_use_constraints.items(): + complement_name, complement_operator, constraint = _extract_constraint_info( + value + ) + + if kwarg in kwargs and complement_operator(server_version, constraint): + msg = ( + f"Use of {kwarg} in {method.__name__} is not permitted " + f"for server version {complement_name}{constraint}, your server " + f"version is {server_version}. Contact your system " + "administrator to update the server version." + ) + raise ValueError(msg) + + out = method(*args, **kwargs) + return out + + return wrapper diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index dea1b889..a26f2aee 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -13,7 +13,13 @@ import pandas as pd from .auth import AuthClient -from .base import BaseEncoder, ClientBase, _api_endpoints, handle_response +from .base import ( + BaseEncoder, + ClientBase, + _api_endpoints, + check_version_compatibility, + handle_response, +) from .endpoints import ( chunkedgraph_api_versions, chunkedgraph_endpoints_common, @@ -819,6 +825,7 @@ def get_subgraph( rd = handle_response(response) return np.int64(rd["nodes"]), np.double(rd["affinities"]), np.int32(rd["areas"]) + @check_version_compatibility(kwarg_use_constraints={"bounds": ">=2.15.0"}) def level2_chunk_graph(self, root_id, bounds=None) -> list: """ Get graph of level 2 chunks, the smallest agglomeration level above supervoxels. From 2c4fcfcfeaf22f2b6d3c14f5912ffcac897be829 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Wed, 27 Mar 2024 16:07:53 -0700 Subject: [PATCH 3/9] way simplify code --- caveclient/base.py | 99 ++++++++++++-------------------------- caveclient/chunkedgraph.py | 51 +++++++------------- 2 files changed, 48 insertions(+), 102 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index ca2a2617..454f9add 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -1,18 +1,17 @@ import datetime import json import logging -import operator +import textwrap import urllib import webbrowser from functools import wraps from typing import Callable import numpy as np +import pandas as pd import requests +from packaging.specifiers import SpecifierSet from packaging.version import Version -from packaging.version import parse as parse_version - -import pandas as pd from .session_config import patch_session @@ -322,55 +321,19 @@ def repl(f): return layer -def _extract_constraint_info(constraint: str) -> tuple[str, Callable, Version]: - """ - Extracts the operator and version number from a version constraint. +class ServerIncompatibilityError(Exception): + def __init__(self, message): + # for readability, wrap the message at 80 characters + message = textwrap.fill(message, width=80) + super().__init__(message) - Parameters - ---------- - constraint - Version constraint described as a comparison operator followed by the version - number. For example, "<=1.0.0" would indicate that this method is only - compatible with server versions less than or equal to 1.0.0. - - Returns - ------- - : - The complement name of the constraint. - : - The complement operator of the constraint. - : - The version object of the constraint. - """ - if "<=" == constraint[:2]: - complement_name = ">" - complement_operator = operator.gt - constraint = constraint[2:] - elif ">=" == constraint[:2]: - complement_name = "<" - complement_operator = operator.lt - constraint = constraint[2:] - elif "==" == constraint[:2]: - complement_name = "!=" - complement_operator = operator.ne - constraint = constraint[2:] - elif "!=" == constraint[:2]: - complement_name = "==" - complement_operator = operator.eq - constraint = constraint[2:] - elif "<" == constraint[0]: - complement_name = ">=" - complement_operator = operator.ge - constraint = constraint[1:] - elif ">" == constraint[0]: - complement_name = "<=" - complement_operator = operator.le - constraint = constraint[1:] +def _version_fails_constraint(version: Version, constraint: str = None): + if constraint is None: + return False else: - raise ValueError(f"Constraint {constraint} not recognized.") - constraint = parse_version(constraint) - return complement_name, complement_operator, constraint + specifier = SpecifierSet(constraint) + return version not in specifier @parametrized @@ -398,40 +361,38 @@ def check_version_compatibility( less than or equal to 1.0.0. An error will be raised only if the user both provides the keyword argument (even if passing in the default value!) and the server version is not compatible with the constraint. + + Raises + ------ + ServerIncompatibilityError + If the server version is not compatible with the constraints. """ @wraps(method) def wrapper(*args, **kwargs): self = args[0] - server_version = parse_version(self.version) if method_constraint is not None: - complement_name, complement_operator, constraint = _extract_constraint_info( - method_constraint - ) - - if complement_operator(server_version, constraint): + if _version_fails_constraint(self.server_version, method_constraint): msg = ( - f"Method {method.__name__} is not permitted " - f"for server version {complement_name}{constraint}, your server " - f"version is {server_version}. Contact your system " + f"Use of method `{method.__name__}` is only permitted " + f"for server version {method_constraint}, your server " + f"version is {self.server_version}. Contact your system " "administrator to update the server version." ) - raise ValueError(msg) - for kwarg, value in kwarg_use_constraints.items(): - complement_name, complement_operator, constraint = _extract_constraint_info( - value - ) + raise ServerIncompatibilityError(msg) - if kwarg in kwargs and complement_operator(server_version, constraint): + for kwarg, kwarg_constraint in kwarg_use_constraints.items(): + if _version_fails_constraint(self.server_version, kwarg_constraint): msg = ( - f"Use of {kwarg} in {method.__name__} is not permitted " - f"for server version {complement_name}{constraint}, your server " - f"version is {server_version}. Contact your system " + f"Use of keyword argument `{kwarg}` in `{method.__name__}` " + "is only permitted " + f"for server version {kwarg_constraint}, your server " + f"version is {self.server_version}. Contact your system " "administrator to update the server version." ) - raise ValueError(msg) + raise ServerIncompatibilityError(msg) out = method(*args, **kwargs) return out diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index a26f2aee..1e9e834b 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -8,9 +8,9 @@ import networkx as nx import numpy as np -import pytz - import pandas as pd +import pytz +from packaging.version import Version from .auth import AuthClient from .base import ( @@ -190,7 +190,7 @@ def __init__( self._default_timestamp = timestamp self._table_name = table_name self._segmentation_info = None - self._semver = self._get_current_semver() + self._server_version = self._get_server_version() @property def default_url_mapping(self): @@ -201,46 +201,29 @@ def table_name(self): return self._table_name # TODO refactor to base client - def _get_current_semver(self): + def _get_server_version(self) -> Optional[Version]: endpoint_mapping = self.default_url_mapping url = self._endpoints["get_current_semver"].format_map(endpoint_mapping) response = self.session.get(url) if response.status_code == 404: # server doesn't have this endpoint yet return None else: - return response.json() - - @property - def major_version(self) -> Optional[int]: - if self._semver is None: - return None - else: - return int(self._semver.split(".")[0]) - - @property - def minor_version(self) -> Optional[int]: - if self._semver is None: - return None - else: - return int(self._semver.split(".")[1]) - - @property - def patch_version(self) -> Optional[int]: - if self._semver is None: - return None - else: - return int(self._semver.split(".")[2]) + version_str = response.json() + version = Version(version_str) + return version @property - def version(self) -> Optional[str]: - return self._semver + def server_version(self) -> Optional[Version]: + return self._server_version @property - def max_version(self) -> str: - if self._semver is None: - return "2.15.0" # this is the last version that doesn't have the endpoints + def max_server_version(self) -> Version: + """Old versions of the server will not even have the endpoint to know""" + if self._server_version is None: + # this is the last version that doesn't have the endpoints + return Version("2.15.0") else: - return self._semver + return self._server_version def _process_timestamp(self, timestamp): """Process timestamp with default logic""" @@ -825,7 +808,9 @@ def get_subgraph( rd = handle_response(response) return np.int64(rd["nodes"]), np.double(rd["affinities"]), np.int32(rd["areas"]) - @check_version_compatibility(kwarg_use_constraints={"bounds": ">=2.15.0"}) + @check_version_compatibility( + method_constraint=">=1.0.0", kwarg_use_constraints={"bounds": ">=2.15.0"} + ) def level2_chunk_graph(self, root_id, bounds=None) -> list: """ Get graph of level 2 chunks, the smallest agglomeration level above supervoxels. From e4eae451ca496f42d3186392a04194038d082c10 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Wed, 27 Mar 2024 16:53:27 -0700 Subject: [PATCH 4/9] code cleanup --- caveclient/base.py | 6 +++--- caveclient/chunkedgraph.py | 11 +++++++---- tests/test_chunkedgraph.py | 8 ++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index 454f9add..99704667 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -337,18 +337,18 @@ def _version_fails_constraint(version: Version, constraint: str = None): @parametrized -def check_version_compatibility( +def _check_version_compatibility( method: Callable, method_constraint: str = None, kwarg_use_constraints: dict = None ) -> Callable: """ - This decorator is used to check the compatibility features in the client and + This decorator is used to check the compatibility of features in the client and server versions. If the server version is not compatible with the constraint, an error will be raised. Parameters ---------- method - Method to be decorated. + Method of the client to be decorated. method_constraint Version constraint for the method, described as a comparison operator followed by the version number. For example, "<=1.0.0" would indicate that this diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index 1e9e834b..82a7b492 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -17,7 +17,7 @@ BaseEncoder, ClientBase, _api_endpoints, - check_version_compatibility, + _check_version_compatibility, handle_response, ) from .endpoints import ( @@ -205,10 +205,13 @@ def _get_server_version(self) -> Optional[Version]: endpoint_mapping = self.default_url_mapping url = self._endpoints["get_current_semver"].format_map(endpoint_mapping) response = self.session.get(url) + print(url) if response.status_code == 404: # server doesn't have this endpoint yet return None else: version_str = response.json() + print(response.json()) + print(response) version = Version(version_str) return version @@ -808,9 +811,7 @@ def get_subgraph( rd = handle_response(response) return np.int64(rd["nodes"]), np.double(rd["affinities"]), np.int32(rd["areas"]) - @check_version_compatibility( - method_constraint=">=1.0.0", kwarg_use_constraints={"bounds": ">=2.15.0"} - ) + @_check_version_compatibility(kwarg_use_constraints={"bounds": ">=2.15.0"}) def level2_chunk_graph(self, root_id, bounds=None) -> list: """ Get graph of level 2 chunks, the smallest agglomeration level above supervoxels. @@ -847,6 +848,8 @@ def level2_chunk_graph(self, root_id, bounds=None) -> list: r = handle_response(response) + # TODO in theory, could remove this check if we are confident in the server + # version fix used_bounds = response.headers.get("Used-Bounds") used_bounds = used_bounds == "true" or used_bounds == "True" if bounds is not None and not used_bounds: diff --git a/tests/test_chunkedgraph.py b/tests/test_chunkedgraph.py index 7bee5cf8..94d939db 100644 --- a/tests/test_chunkedgraph.py +++ b/tests/test_chunkedgraph.py @@ -8,6 +8,7 @@ import responses from responses.matchers import json_params_matcher +from caveclient.base import ServerIncompatibilityError from caveclient.endpoints import ( chunkedgraph_endpoints_common, chunkedgraph_endpoints_v1, @@ -420,6 +421,13 @@ def test_get_lvl2subgraph_bounds(self, myclient): root_id, bounds=bounds ) + def test_lvl2subgraph_old_server_fails_bounds(self, myclient): + myclient.chunkedgraph._version = "0.1.0" + with pytest.raises(ServerIncompatibilityError): + myclient.chunkedgraph.level2_chunk_graph( + 0, bounds=np.array([[0, 1], [0, 1], [0, 1]]) + ) + @responses.activate def test_get_remeshing(self, myclient): endpoint_mapping = self._default_endpoint_map From 084f884345064e9b385ed7e4bfea02695007f288 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Thu, 28 Mar 2024 11:15:47 -0700 Subject: [PATCH 5/9] fix tests --- caveclient/base.py | 1 + caveclient/chunkedgraph.py | 15 ++++++++------- caveclient/endpoints.py | 2 +- tests/conftest.py | 34 +++++++++++++++++++++++++++++++--- tests/test_chunkedgraph.py | 36 ++++++++++++++++++++++++++++++++---- 5 files changed, 73 insertions(+), 15 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index 99704667..3ad8e74d 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -91,6 +91,7 @@ def _raise_for_status(r, log_warning=True): def handle_response(response, as_json=True, log_warning=True): """Deal with potential errors in endpoint response and return json for default case""" + # NOTE: consider adding "None on 404" as an option? _raise_for_status(response, log_warning=log_warning) _check_authorization_redirect(response) if as_json: diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index 82a7b492..76b83cbe 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -190,7 +190,7 @@ def __init__( self._default_timestamp = timestamp self._table_name = table_name self._segmentation_info = None - self._server_version = self._get_server_version() + self._server_version = self._get_version() @property def default_url_mapping(self): @@ -201,27 +201,28 @@ def table_name(self): return self._table_name # TODO refactor to base client - def _get_server_version(self) -> Optional[Version]: + def _get_version(self) -> Optional[Version]: endpoint_mapping = self.default_url_mapping - url = self._endpoints["get_current_semver"].format_map(endpoint_mapping) + url = self._endpoints["get_version"].format_map(endpoint_mapping) response = self.session.get(url) - print(url) if response.status_code == 404: # server doesn't have this endpoint yet return None else: version_str = response.json() - print(response.json()) - print(response) version = Version(version_str) return version @property def server_version(self) -> Optional[Version]: + """The version of the remote server.""" return self._server_version @property def max_server_version(self) -> Version: - """Old versions of the server will not even have the endpoint to know""" + """ + The version of the remote server, or the last version of the server which + does not have the endpoint for displaying its version if unavailable. + """ if self._server_version is None: # this is the last version that doesn't have the endpoints return Version("2.15.0") diff --git a/caveclient/endpoints.py b/caveclient/endpoints.py index 33e2e1f5..39d98b69 100644 --- a/caveclient/endpoints.py +++ b/caveclient/endpoints.py @@ -128,7 +128,7 @@ pcg_common = "{cg_server_address}/segmentation" chunkedgraph_endpoints_common = { "get_api_versions": pcg_common + "/api/versions", - "get_current_semver": pcg_common + "/api/current_semver", + "get_version": pcg_common + "/api/version", "info": pcg_common + "/table/{table_id}/info", } diff --git a/tests/conftest.py b/tests/conftest.py index 7fbf89b7..56efa1c0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,18 +27,46 @@ "viewer_resolution_y": 4.0, "viewer_resolution_z": 40, } +url_template = endpoints.infoservice_endpoints_v2["datastack_info"] +mapping = {"i_server_address": TEST_GLOBAL_SERVER, "datastack_name": TEST_DATASTACK} +url = url_template.format_map(mapping) @pytest.fixture() @responses.activate def myclient(): - url_template = endpoints.infoservice_endpoints_v2["datastack_info"] - mapping = {"i_server_address": TEST_GLOBAL_SERVER, "datastack_name": TEST_DATASTACK} - url = url_template.format_map(mapping) + responses.add(responses.GET, url, json=test_info, status=200) + + client = CAVEclient( + TEST_DATASTACK, server_address=TEST_GLOBAL_SERVER, write_server_cache=False + ) + + # need to mock the response of the version checking code for each sub-client which + # wants that information, and then create the sub-client here since the mock is + # narrowly scoped to this function + version_url = f"{TEST_LOCAL_SERVER}/segmentation/api/version" + responses.add(responses.GET, version_url, json="2.15.0", status=200) + client.chunkedgraph # this will trigger the version check + + return client + + +@pytest.fixture() +@responses.activate +def old_chunkedgraph_client(): responses.add(responses.GET, url, json=test_info, status=200) client = CAVEclient( TEST_DATASTACK, server_address=TEST_GLOBAL_SERVER, write_server_cache=False ) + + # need to mock the response of the version checking code for each sub-client which + # wants that information, and then create the sub-client here since the mock is + # narrowly scoped to this function + version_url = f"{TEST_LOCAL_SERVER}/segmentation/api/version" + responses.add(responses.GET, version_url, json="1.0.0", status=200) + + client.chunkedgraph # this will trigger the version check + return client diff --git a/tests/test_chunkedgraph.py b/tests/test_chunkedgraph.py index 94d939db..87540415 100644 --- a/tests/test_chunkedgraph.py +++ b/tests/test_chunkedgraph.py @@ -1,5 +1,6 @@ import datetime import json +import os from urllib.parse import urlencode import numpy as np @@ -16,6 +17,31 @@ from .conftest import TEST_LOCAL_SERVER, test_info +TEST_GLOBAL_SERVER = os.environ.get("TEST_SERVER", "https://test.cave.com") +TEST_LOCAL_SERVER = os.environ.get("TEST_LOCAL_SERVER", "https://local.cave.com") +TEST_DATASTACK = os.environ.get("TEST_DATASTACK", "test_stack") + +test_info = { + "viewer_site": "http://neuromancer-seung-import.appspot.com/", + "aligned_volume": { + "name": "test_volume", + "image_source": f"precomputed://https://{TEST_LOCAL_SERVER}/test-em/v1", + "id": 1, + "description": "This is a test only dataset.", + }, + "synapse_table": "test_synapse_table", + "description": "This is the first test datastack. ", + "local_server": TEST_LOCAL_SERVER, + "segmentation_source": f"graphene://https://{TEST_LOCAL_SERVER}/segmentation/table/test_v1", + "soma_table": "test_soma", + "analysis_database": None, + "viewer_resolution_x": 4.0, + "viewer_resolution_y": 4.0, + "viewer_resolution_z": 40, +} +# version_url = f"{TEST_LOCAL_SERVER}/segmentation/api/current_semver" +# responses.add(responses.GET, version_url, json="1.0.0", status=200) + def binary_body_match(body): def match(request_body): @@ -421,11 +447,13 @@ def test_get_lvl2subgraph_bounds(self, myclient): root_id, bounds=bounds ) - def test_lvl2subgraph_old_server_fails_bounds(self, myclient): - myclient.chunkedgraph._version = "0.1.0" + @responses.activate + def test_lvl2subgraph_old_server_fails_bounds(self, old_chunkedgraph_client): + bounds = np.array([[83875, 85125], [82429, 83679], [20634, 20884]]) + root_id = 864691136812623475 with pytest.raises(ServerIncompatibilityError): - myclient.chunkedgraph.level2_chunk_graph( - 0, bounds=np.array([[0, 1], [0, 1], [0, 1]]) + old_chunkedgraph_client.chunkedgraph.level2_chunk_graph( + root_id, bounds=bounds ) @responses.activate From 4a7f9f0a45d92e1eb35ecf6bb9de9432ce9dca91 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Thu, 28 Mar 2024 11:17:00 -0700 Subject: [PATCH 6/9] fix redef --- tests/test_chunkedgraph.py | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/tests/test_chunkedgraph.py b/tests/test_chunkedgraph.py index 87540415..f0fccb22 100644 --- a/tests/test_chunkedgraph.py +++ b/tests/test_chunkedgraph.py @@ -1,6 +1,5 @@ import datetime import json -import os from urllib.parse import urlencode import numpy as np @@ -17,31 +16,6 @@ from .conftest import TEST_LOCAL_SERVER, test_info -TEST_GLOBAL_SERVER = os.environ.get("TEST_SERVER", "https://test.cave.com") -TEST_LOCAL_SERVER = os.environ.get("TEST_LOCAL_SERVER", "https://local.cave.com") -TEST_DATASTACK = os.environ.get("TEST_DATASTACK", "test_stack") - -test_info = { - "viewer_site": "http://neuromancer-seung-import.appspot.com/", - "aligned_volume": { - "name": "test_volume", - "image_source": f"precomputed://https://{TEST_LOCAL_SERVER}/test-em/v1", - "id": 1, - "description": "This is a test only dataset.", - }, - "synapse_table": "test_synapse_table", - "description": "This is the first test datastack. ", - "local_server": TEST_LOCAL_SERVER, - "segmentation_source": f"graphene://https://{TEST_LOCAL_SERVER}/segmentation/table/test_v1", - "soma_table": "test_soma", - "analysis_database": None, - "viewer_resolution_x": 4.0, - "viewer_resolution_y": 4.0, - "viewer_resolution_z": 40, -} -# version_url = f"{TEST_LOCAL_SERVER}/segmentation/api/current_semver" -# responses.add(responses.GET, version_url, json="1.0.0", status=200) - def binary_body_match(body): def match(request_body): From 985c42c708ca9241465c505d6b4fb1d2ffc11364 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Thu, 28 Mar 2024 11:22:14 -0700 Subject: [PATCH 7/9] fix formatting --- caveclient/base.py | 2 +- caveclient/chunkedgraph.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index 3ad8e74d..1e275948 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -91,7 +91,7 @@ def _raise_for_status(r, log_warning=True): def handle_response(response, as_json=True, log_warning=True): """Deal with potential errors in endpoint response and return json for default case""" - # NOTE: consider adding "None on 404" as an option? + # NOTE: consider adding "None on 404" as an option? _raise_for_status(response, log_warning=log_warning) _check_authorization_redirect(response) if as_json: diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index 76b83cbe..e3724f66 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -220,7 +220,7 @@ def server_version(self) -> Optional[Version]: @property def max_server_version(self) -> Version: """ - The version of the remote server, or the last version of the server which + The version of the remote server, or the last version of the server which does not have the endpoint for displaying its version if unavailable. """ if self._server_version is None: From 5b23a402e1c2e93e4989ff039a31db9aa1b3618a Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Tue, 2 Apr 2024 15:53:53 -0700 Subject: [PATCH 8/9] refactor --- caveclient/base.py | 20 +++++++++++++++++++- caveclient/chunkedgraph.py | 29 +---------------------------- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index 1e275948..c0ec83f8 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -5,7 +5,7 @@ import urllib import webbrowser from functools import wraps -from typing import Callable +from typing import Callable, Optional import numpy as np import pandas as pd @@ -232,6 +232,24 @@ def server_address(self): def api_version(self): return self._api_version + def _get_version(self) -> Optional[Version]: + endpoint_mapping = self.default_url_mapping + url = self._endpoints["get_version"].format_map(endpoint_mapping) + response = self.session.get(url) + # TODO none_on_404 is not a thing + response = handle_response(response, as_json=True, none_on_404=True) + # if response.status_code == 404: # server doesn't have this endpoint yet + # return None + # else: + # version_str = response.json() + # version = Version(version_str) + # return version + + @property + def server_version(self) -> Optional[Version]: + """The version of the remote server.""" + return self._server_version + @staticmethod def raise_for_status(r, log_warning=True): """Raises [requests.HTTPError][], if one occurred.""" diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index e3724f66..74a660fb 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -200,34 +200,7 @@ def default_url_mapping(self): def table_name(self): return self._table_name - # TODO refactor to base client - def _get_version(self) -> Optional[Version]: - endpoint_mapping = self.default_url_mapping - url = self._endpoints["get_version"].format_map(endpoint_mapping) - response = self.session.get(url) - if response.status_code == 404: # server doesn't have this endpoint yet - return None - else: - version_str = response.json() - version = Version(version_str) - return version - - @property - def server_version(self) -> Optional[Version]: - """The version of the remote server.""" - return self._server_version - - @property - def max_server_version(self) -> Version: - """ - The version of the remote server, or the last version of the server which - does not have the endpoint for displaying its version if unavailable. - """ - if self._server_version is None: - # this is the last version that doesn't have the endpoints - return Version("2.15.0") - else: - return self._server_version + def _process_timestamp(self, timestamp): """Process timestamp with default logic""" From 2bbca71475e4c302eaf9678b52260f754aeb0f92 Mon Sep 17 00:00:00 2001 From: Ben Pedigo Date: Wed, 3 Apr 2024 09:18:03 -0700 Subject: [PATCH 9/9] protect for missing endpoint --- caveclient/base.py | 16 +++++++--------- caveclient/chunkedgraph.py | 5 +---- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/caveclient/base.py b/caveclient/base.py index c0ec83f8..054a1d13 100644 --- a/caveclient/base.py +++ b/caveclient/base.py @@ -234,16 +234,14 @@ def api_version(self): def _get_version(self) -> Optional[Version]: endpoint_mapping = self.default_url_mapping - url = self._endpoints["get_version"].format_map(endpoint_mapping) + url = self._endpoints.get("get_version", None).format_map(endpoint_mapping) response = self.session.get(url) - # TODO none_on_404 is not a thing - response = handle_response(response, as_json=True, none_on_404=True) - # if response.status_code == 404: # server doesn't have this endpoint yet - # return None - # else: - # version_str = response.json() - # version = Version(version_str) - # return version + if response.status_code == 404: # server doesn't have this endpoint yet + return None + else: + version_str = handle_response(response, as_json=True) + version = Version(version_str) + return version @property def server_version(self) -> Optional[Version]: diff --git a/caveclient/chunkedgraph.py b/caveclient/chunkedgraph.py index 74a660fb..8fa7c8bf 100644 --- a/caveclient/chunkedgraph.py +++ b/caveclient/chunkedgraph.py @@ -3,14 +3,13 @@ import datetime import json import logging -from typing import Iterable, Optional, Tuple, Union +from typing import Iterable, Tuple, Union from urllib.parse import urlencode import networkx as nx import numpy as np import pandas as pd import pytz -from packaging.version import Version from .auth import AuthClient from .base import ( @@ -200,8 +199,6 @@ def default_url_mapping(self): def table_name(self): return self._table_name - - def _process_timestamp(self, timestamp): """Process timestamp with default logic""" if timestamp is None: