Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
56 changes: 54 additions & 2 deletions caveclient/materializationengine.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
)
else:
raise ValueError(
f'Unknown response type: {response.headers.get("Content-Type")}'
f"Unknown response type: {response.headers.get('Content-Type')}"
)


Expand Down Expand Up @@ -228,6 +228,12 @@
self._tables = None
self._views = None

@property
@cached(cache=TTLCache(maxsize=1, ttl=60 * 60 * 1))
def available_versions(self) -> list[int]:
"""Get the available versions for this materialization client."""
return sorted(self.get_versions(expired=False))

@property
def datastack_name(self):
"""The name of the datastack."""
Expand Down Expand Up @@ -361,6 +367,10 @@
datastack_name = self.datastack_name
if version is None:
version = self.version
if version not in self.available_versions:
raise ValueError(

Check warning on line 371 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L370-L371

Added lines #L370 - L371 were not covered by tests
f"Annotation count must use a materialized version ({self.available_versions})."
)
endpoint_mapping = self.default_url_mapping
endpoint_mapping["datastack_name"] = datastack_name
endpoint_mapping["table_name"] = table_name
Expand Down Expand Up @@ -406,6 +416,7 @@
d["expires_on"] = convert_timestamp(d["expires_on"])
return d

@cached(cache=TTLCache(maxsize=50, ttl=60 * 60 * 24))
def get_timestamp(
self, version: Optional[int] = None, datastack_name: str = None
) -> datetime:
Expand Down Expand Up @@ -741,6 +752,14 @@

if desired_resolution is None:
desired_resolution = self.desired_resolution
if (
materialization_version not in self.available_versions
and materialization_version is not None
):
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting version validation and fallback logic into a shared helper to reduce duplication and maintain consistent behavior across query methods.

Copilot uses AI. Check for mistakes.
timestamp = self.get_timestamp(

Check warning on line 759 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L759

Added line #L759 was not covered by tests
version=materialization_version, datastack_name=datastack_name
)
materialization_version = None

Check warning on line 762 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L762

Added line #L762 was not covered by tests
if timestamp is not None:
if materialization_version is not None:
raise ValueError("cannot specify timestamp and materialization version")
Expand Down Expand Up @@ -975,6 +994,10 @@

if materialization_version is None:
materialization_version = self.version
if materialization_version not in self.available_versions:
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] In join_query, expired versions raise an error while query_table falls back to a timestamp-based query; consider aligning these behaviors or centralizing the fallback logic.

Copilot uses AI. Check for mistakes.
raise ValueError(

Check warning on line 998 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L998

Added line #L998 was not covered by tests
f"Cannot use `join_query` for a non-materialized version. Please use a materialized version ({self.available_versions}) or live_live_query."
)
if datastack_name is None:
datastack_name = self.datastack_name
if desired_resolution is None:
Expand Down Expand Up @@ -1178,7 +1201,7 @@
all_svid_lengths.append(n_svids)
logger.info(f"{sv_col} has {n_svids} to update")
all_svids = np.append(all_svids, svids[~is_latest_root])
logger.info(f"num zero svids: {np.sum(all_svids==0)}")
logger.info(f"num zero svids: {np.sum(all_svids == 0)}")
logger.info(f"all_svids dtype {all_svids.dtype}")
logger.info(f"all_svid_lengths {all_svid_lengths}")
with MyTimeIt("get_roots"):
Expand Down Expand Up @@ -2225,6 +2248,11 @@
datastack_name = self.datastack_name
if version is None:
version = self.version
if version not in self.available_versions:
raise ValueError(

Check warning on line 2252 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L2252

Added line #L2252 was not covered by tests
f"Materialization version must not be expired for views. "
f"Available versions: {self.available_versions}"
)
Copy link

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] These expiry checks are duplicated in several methods (get_views, get_view_metadata, etc.); refactor into a common validator to improve maintainability.

Suggested change
if version not in self.available_versions:
raise ValueError(
f"Materialization version must not be expired for views. "
f"Available versions: {self.available_versions}"
)
self.validate_version(version)

Copilot uses AI. Check for mistakes.
endpoint_mapping = self.default_url_mapping
endpoint_mapping["datastack_name"] = datastack_name
endpoint_mapping["version"] = version
Expand Down Expand Up @@ -2262,6 +2290,14 @@
datastack_name = self.datastack_name
if materialization_version is None:
materialization_version = self.version
if (
materialization_version not in self.available_versions
and materialization_version is not None
):
raise ValueError(

Check warning on line 2297 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L2297

Added line #L2297 was not covered by tests
f"Materialization version must not be expired for view metadata query. "
f"Available versions: {self.available_versions}"
)

endpoint_mapping = self.default_url_mapping
endpoint_mapping["view_name"] = view_name
Expand Down Expand Up @@ -2338,6 +2374,14 @@
datastack_name = self.datastack_name
if materialization_version is None:
materialization_version = self.version
if (
materialization_version not in self.available_versions
and materialization_version is not None
):
raise ValueError(

Check warning on line 2381 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L2381

Added line #L2381 was not covered by tests
f"Materialization version must not be expired for view schema query. "
f"Available versions: {self.available_versions}"
)

endpoint_mapping = self.default_url_mapping
endpoint_mapping["datastack_name"] = datastack_name
Expand Down Expand Up @@ -2452,6 +2496,14 @@
materialization_version = self.version
if datastack_name is None:
datastack_name = self.datastack_name
if (

Check warning on line 2499 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L2499

Added line #L2499 was not covered by tests
materialization_version not in self.available_versions
and materialization_version is not None
):
raise ValueError(

Check warning on line 2503 in caveclient/materializationengine.py

View check run for this annotation

Codecov / codecov/patch

caveclient/materializationengine.py#L2503

Added line #L2503 was not covered by tests
f"Materialization version must not be expired for view query. "
f"Available versions: {self.available_versions}"
)

url, data, query_args, encoding = self._format_query_components(
datastack_name,
Expand Down
9 changes: 8 additions & 1 deletion caveclient/tools/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,6 @@ def mockedCAVEclient():
status=200,
match=[query_param_matcher({"expired": True})],
)

if set_version is not None:
mat_mapping["version"] = set_version
version_metadata_url = mat_endpoints["version_metadata"].format_map(
Expand Down Expand Up @@ -533,6 +532,14 @@ def mockedCAVEclient():
client.chunkedgraph
if materialization:
client.materialize
responses.add(
responses.GET,
mat_version_list_url,
json=available_materialization_versions,
status=200,
match=[query_param_matcher({"expired": False})],
)
client.materialize.available_versions
if json_service:
client.state
if skeleton_service:
Expand Down