RSDK-2870: use api instead of subtype #824
Conversation
|
Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!
|
src/viam/logging.py
Outdated
| @@ -83,7 +83,7 @@ async def handle_task_result(self, task: asyncio.Task): | |||
|
|
|||
| def emit(self, record: logging.LogRecord): | |||
| assert isinstance(record, logging.LogRecord) | |||
| # Fully qualified name of form "{subtype triplet}/{name}", e.g. "rdk:component:arm/myarm" | |||
| # Fully qualified name of form "{API}/{name}", e.g. "rdk:component:arm/myarm" | |||
There was a problem hiding this comment.
(minor, q) curious why you got rid of "triplet" here? Not a big deal either way, but I think it might be helpful to keep since the way we're using API is a bit of a term of art.
There was a problem hiding this comment.
There was no real reason to get rid of the word "triplet". In other comments, it's referred to simply as API so it was what I did automatically. I can add it back no problem!
src/viam/app/data_client.py
Outdated
| resource_subtype: str | ||
| """ | ||
| deprecated; use `resource_api` | ||
| The resource API. Ex: `rdk:component:sensor` | ||
| """ |
There was a problem hiding this comment.
(minor) I'd love for this to be a property instead, so people can't set it, it's impossible for it to ever drift from the value in resource_api, and so we can emit a deprecation warning on call instead of just including it in comments. Something like
@property
def resource_subtype(self) -> str:
warnings.warn("<some deprecation warning>", DeprecationWarning, stacklevel=2)
return self.resource_api
src/viam/app/data_client.py
Outdated
| @@ -461,6 +469,7 @@ async def export_tabular_data( | |||
| DataClient.TabularDataPoint( | |||
| part_id=resp.part_id, | |||
| resource_name=resp.resource_name, | |||
| resource_subtype=resp.resource_subtype, | |||
There was a problem hiding this comment.
given above comment re: property decorator, I don't think we need to (or should) do this setting here.
subtypewhen actually referring to the API triplet (i.e.rdk:component:sensor)DataClientfunctions and has a deprecated comment inTabularDataPointImportant
BREAKING CHANGES INTRODUCED IN THIS PR
SUBTYPEis nowAPIRegistrymethods have been renamed or contain parameter name changesNote for reviewers: I tried my best to go through all the uses of
subtypethat I could find. Because this is such a loosely used word, it was hard to figure out what was actually asubtypeand what should be anAPIat times, which is the whole point of the change. I wouldn't be surprised if I missed something