Skip to content

Commit

Permalink
Fix MaterializationEngine version constraints (#250)
Browse files Browse the repository at this point in the history
* fix constraints

* formatting

* add str

* fix version spec

* fix api version
  • Loading branch information
bdpedigo authored Oct 15, 2024
1 parent 91619ad commit cc1d9cf
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 18 deletions.
49 changes: 48 additions & 1 deletion caveclient/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,11 @@ def _version_fails_constraint(

@parametrized
def _check_version_compatibility(
method: Callable, method_constraint: str = None, kwarg_use_constraints: dict = None
method: Callable,
method_constraint: str = None,
kwarg_use_constraints: dict = None,
method_api_constraint: str = None,
kwarg_use_api_constraints: dict = None,
) -> Callable:
"""
This decorator is used to check the compatibility of features in the client and
Expand Down Expand Up @@ -469,6 +473,49 @@ def wrapper(*args, **kwargs):
)
raise ServerIncompatibilityError(msg)

if method_api_constraint is not None:
if _version_fails_constraint(
Version(str(self.api_version)), method_api_constraint
):
if isinstance(method_api_constraint, str):
ms_list = [method_api_constraint]
else:
ms_list = method_api_constraint
msg = (
f"Use of method `{method.__name__}` is only permitted "
f"for API version {' or '.join(ms_list)}. Your API "
f"version is {self.api_version}. Contact your system "
"administrator to update the API version."
)

raise ServerIncompatibilityError(msg)

if kwarg_use_api_constraints is not None:
# this protects against someone passing in a positional argument for the
# kwarg we are guarding
check_kwargs = kwargs.copy()
# add in any positional arguments that are not self to the checking
check_kwargs.update(zip(method.__code__.co_varnames[1:], args[1:]))

for kwarg, kwarg_constraint in kwarg_use_api_constraints.items():
if check_kwargs.get(kwarg, None) is None:
continue
elif _version_fails_constraint(
Version(str(self.api_version)), kwarg_constraint
):
if isinstance(kwarg_constraint, str):
kw_list = [kwarg_constraint]
else:
kw_list = kwarg_constraint
msg = (
f"Use of keyword argument `{kwarg}` in `{method.__name__}` "
"is only permitted "
f"for API version {' or '.join(kw_list)}. Your API "
f"version is {self.api_version}. Contact your system "
"administrator to update the API version."
)
raise ServerIncompatibilityError(msg)

out = method(*args, **kwargs)
return out

Expand Down
36 changes: 19 additions & 17 deletions caveclient/materializationengine.py
Original file line number Diff line number Diff line change
Expand Up @@ -1804,7 +1804,7 @@ def tables(self) -> TableManager:
"""The table manager for the materialization engine."""
if self._tables is None:
if self.fc is not None and self.fc._materialize is not None:
if self.server_version < Version("3"):
if Version(str(self.api_version)) < Version("3"):
tables = TableManager(self.fc)
else:
metadata = []
Expand Down Expand Up @@ -1836,7 +1836,7 @@ def tables(self) -> TableManager:
def views(self) -> ViewManager:
"""The view manager for the materialization engine."""
if self.fc is not None and self.fc._materialize is not None:
if self.server_version < Version("3"):
if Version(str(self.api_version)) < Version("3"):
views = ViewManager(self.fc)
else:
metadata = []
Expand All @@ -1855,7 +1855,7 @@ def views(self) -> ViewManager:
raise ValueError("No full CAVEclient specified")
return self._views

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
@cached(cache=TTLCache(maxsize=100, ttl=60 * 60 * 12), key=_tables_metadata_key)
def get_tables_metadata(
self,
Expand Down Expand Up @@ -1901,13 +1901,15 @@ def get_tables_metadata(

@_check_version_compatibility(
kwarg_use_constraints={
"filter_regex_dict": ">=3.0.0",
"allow_invalid_root_ids": ">=3.0.0",
"filter_greater_dict": ">=4.34.0",
"filter_less_dict": ">=4.34.0",
"filter_greater_equal_dict": ">=4.34.0",
"filter_less_equal_dict": ">=4.34.0",
}
},
kwarg_use_api_constraints={
"filter_regex_dict": ">=3.0.0",
"allow_invalid_root_ids": ">=3.0.0",
},
)
def live_live_query(
self,
Expand Down Expand Up @@ -2117,7 +2119,7 @@ def live_live_query(
data["suffixes"] = suffixes
if desired_resolution is None:
desired_resolution = self.desired_resolution
if self.server_version >= Version("3"):
if Version(str(self.api_version)) >= Version("3"):
if desired_resolution is not None:
data["desired_resolution"] = desired_resolution
encoding = DEFAULT_COMPRESSION
Expand All @@ -2141,9 +2143,9 @@ def live_live_query(
warnings.simplefilter(action="ignore", category=DeprecationWarning)
df = deserialize_query_response(response)
if desired_resolution is not None:
if self.server_version < Version("3") or not response.headers.get(
"dataframe_resolution", None
):
if Version(str(self.api_version)) < Version(
"3"
) or not response.headers.get("dataframe_resolution", None):
if len(desired_resolution) != 3:
raise ValueError(
"desired resolution needs to be of length 3, for xyz"
Expand All @@ -2169,7 +2171,7 @@ def live_live_query(
"less_equal": filter_less_equal_dict,
"spatial": filter_spatial_dict,
}
if self.server_version < Version("3"):
if Version(str(self.api_version)) < Version("3"):
_desired_resolution = desired_resolution
else:
_desired_resolution = response.headers.get(
Expand Down Expand Up @@ -2197,7 +2199,7 @@ def live_live_query(
)
return df

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
def get_views(self, version: Optional[int] = None, datastack_name: str = None):
"""
Get all available views for a version
Expand Down Expand Up @@ -2227,7 +2229,7 @@ def get_views(self, version: Optional[int] = None, datastack_name: str = None):
self.raise_for_status(response)
return response.json()

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
def get_view_metadata(
self,
view_name: str,
Expand Down Expand Up @@ -2267,7 +2269,7 @@ def get_view_metadata(
self.raise_for_status(response, log_warning=log_warning)
return response.json()

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
def get_view_schema(
self,
view_name: str,
Expand Down Expand Up @@ -2307,7 +2309,7 @@ def get_view_schema(
self.raise_for_status(response, log_warning=log_warning)
return response.json()

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
def get_view_schemas(
self,
materialization_version: Optional[int] = None,
Expand Down Expand Up @@ -2343,7 +2345,7 @@ def get_view_schemas(
return response.json()

@_check_version_compatibility(
method_constraint=">=3.0.0",
method_api_constraint=">=3.0.0",
kwarg_use_constraints={
"filter_greater_dict": ">=4.34.0",
"filter_less_dict": ">=4.34.0",
Expand Down Expand Up @@ -2533,7 +2535,7 @@ def query_view(
else:
return response.json()

@_check_version_compatibility(method_constraint=">=3.0.0")
@_check_version_compatibility(method_api_constraint=">=3.0.0")
def get_unique_string_values(
self, table: str, datastack_name: Optional[str] = None
):
Expand Down

0 comments on commit cc1d9cf

Please sign in to comment.