RSDK-7732 EasyResource mixin #643
Conversation
src/viam/resource/easy_resource.py
Outdated
| won't call it directly. | ||
| """ | ||
| logger.debug('registering %s %s', cls.SUBTYPE, cls.MODEL) | ||
| # note: this would pass if EasyResource inherited ResourceBase, but that crashes in the mro() walk in ResourceManager.register. |
There was a problem hiding this comment.
What does this comment mean? What would "pass"?
There was a problem hiding this comment.
pass pyright I mean (not tests)
the ResourceCreatorRegistration wants cls.new to return a ResourceBase
for a while I had ResourceBase as a base class for EasyResource, but it crashed in the mro walk here
could potentially tweak the types / stubs a bit to get this working without the pyright-ignore
| module = cls(args.socket_path, log_level=args.log_level) | ||
| for model in models: | ||
| # todo(review): the pyright ignore below is for ResourceBase.MODEL. Is this not a required field? Should I not rely on it? | ||
| module.add_model_from_registry(model.SUBTYPE, model.MODEL) # pyright: ignore [reportAttributeAccessIssue] |
There was a problem hiding this comment.
ResourceBase does not have a MODEL field at all, since MODELs are for implementation of a ResourceBase, and ResourceBase is for creating the resource definition/interface. This would fail in the possible event that someone passes Arm as the param instead of MyModuleArm
There was a problem hiding this comment.
So would be useful to maybe do a hasattr here and fail if someone doesn't pass the correct type. And also add that disclaimer to the docstring
There was a problem hiding this comment.
added hasattr check w/ helpful error
I didn't expand docstring because when I started writing it I was saying 'the provided resources have to be complete + runnable' and it felt weird. but can add something if you think it's helpful
src/viam/module/module.py
Outdated
| args = parse_module_args() | ||
| module = cls(args.socket_path, log_level=args.log_level) | ||
| for key in Registry.REGISTERED_RESOURCE_CREATORS().keys(): | ||
| # todo(review): this would be cleaner if resource creator key became a tuple |
There was a problem hiding this comment.
I agree, seems like it'd be easy but it is technically a breaking change, but I don't know anyone who would be using the Registry externally like that
There was a problem hiding this comment.
non-breaking middle ground: REGISTERED_RESOURCE_CREATORS() transforms the tuple key to a string key, have another getter that provides access to the tuple?
| module = cls(args.socket_path, log_level=args.log_level) | ||
| for key in Registry.REGISTERED_RESOURCE_CREATORS().keys(): | ||
| # todo(review): this would be cleaner if resource creator key became a tuple | ||
| module.add_model_from_registry(*key.split('/')) # pyright: ignore [reportArgumentType] |
There was a problem hiding this comment.
Dislike the disable pyright here but also seems dumb to create a Subtype just to stringify it again in the next method call. No action needed, just writing it down
What changed
EasyResourcemixin class for shorter model classesrun_from_registryandrun_with_modelsentrypoints inModuleclassWhy
Reduce friction to create new models + get started with our platform.
Checklist
todo(review)comments