Data-3395: Add GetLatestTabularData #793
Conversation
This reverts commit d6b1d80. modified: src/viam/app/data_client.py modified: src/viam/version_metadata.py modified: tests/mocks/services.py modified: tests/test_data_client.py
src/viam/app/data_client.py
Outdated
| @@ -306,6 +308,39 @@ async def tabular_data_by_mql(self, organization_id: str, mql_binary: List[bytes | |||
| response: TabularDataByMQLResponse = await self._data_client.TabularDataByMQL(request, metadata=self._metadata) | |||
| return [bson.decode(bson_bytes) for bson_bytes in response.raw_data] | |||
|
|
|||
| async def get_latest_tabular_data(self, part_id: str, resource_name: str, resource_subtype: str, method_name: str) -> Tuple[ Optional[datetime], Optional[datetime], Dict[str, ValueTypes]]: | |||
There was a problem hiding this comment.
Since some of the return values are optional, I think they should be moved to be after the non-optional value in the tuple
src/viam/app/data_client.py
Outdated
| Tuple[Dict[str, ValueTypes], Optional[datetime], Optional[datetime]: A tuple containing the following: | ||
| Optional[datetime]: The time captured, | ||
| Optional[datetime]: The time synced, | ||
| Dict[str, ValueTypes]: The latest tabular data captured from the specified data source. |
There was a problem hiding this comment.
rearrange these return types to match the order they are returned in the tuple
|
|
||
| time_captured, time_synced, payload = await data_client.get_latest_tabular_data( | ||
| part_id="<PART-ID>", | ||
| resource_name="<RESOURCE-NAME>", |
src/viam/app/data_client.py
Outdated
| request = GetLatestTabularDataRequest(part_id=part_id, resource_name=resource_name, resource_subtype=resource_subtype, method_name=method_name) | ||
| response: GetLatestTabularDataResponse = await self._data_client.GetLatestTabularData(request, metadata=self._metadata) | ||
| if not response.payload: | ||
| return None, None, {} |
There was a problem hiding this comment.
If we get an empty GetLatestTabularDataResponse (aka data hasn't been synced yet for the data source) could we return None for the whole return tuple? I don't like how only the time_captured and time_synced fields are optional and the payload isn't, since those are associated with the payload and should only be returned when the payload is also present.
I'm not very familiar with python - so let me know if this is not super python-y!
There was a problem hiding this comment.
yes makes sense, i changed the entire tuple to be optional so we can return None
| @@ -306,6 +308,40 @@ async def tabular_data_by_mql(self, organization_id: str, mql_binary: List[bytes | |||
| response: TabularDataByMQLResponse = await self._data_client.TabularDataByMQL(request, metadata=self._metadata) | |||
| return [bson.decode(bson_bytes) for bson_bytes in response.raw_data] | |||
|
|
|||
| async def get_latest_tabular_data(self, part_id: str, resource_name: str, resource_subtype: str, method_name: str) -> Optional[Tuple[datetime,datetime, Dict[str, ValueTypes]]]: | |||
There was a problem hiding this comment.
| async def get_latest_tabular_data(self, part_id: str, resource_name: str, resource_subtype: str, method_name: str) -> Optional[Tuple[datetime,datetime, Dict[str, ValueTypes]]]: | |
| async def get_latest_tabular_data(self, part_id: str, resource_name: str, resource_subtype: str, method_name: str) -> Optional[Tuple[datetime, datetime, Dict[str, ValueTypes]]]: |
Just a simple whitespace thing
| data=dict_to_struct(self.tabular_response[0].data) | ||
| await stream.send_message(GetLatestTabularDataResponse(payload=data, time_captured=timestamp, time_synced=timestamp)) | ||
|
|
||
|
|
There was a problem hiding this comment.
I suspect this is one too many new lines added
| method_name (str): The data capture method name. | ||
|
|
||
| Returns: | ||
| Optional[Tuple[Dict[str, ValueTypes], datetime, datetime]: A tuple which is None data hasn't been synced yet for the data source, otherwise the tuple contains the following: |
There was a problem hiding this comment.
Reorder this tuple to be the actual return order
We added a new Data endpoint GetLatestTabularData here, this PR adds the endpoint to python SDK.