Skip to content

RSDK-7732 EasyResource mixin #643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jun 14, 2024
Merged

RSDK-7732 EasyResource mixin #643

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9ff8e7f
EasyResource base class working
abe-winter Jun 12, 2024
b664569
fix model parser, add entrypoints
abe-winter Jun 12, 2024
01abba5
don't inherit ResourceBase -- it crashes at mro walk in ResourceManag…
abe-winter Jun 12, 2024
5b44f1c
easy_resource example + pyinstaller
abe-winter Jun 12, 2024
318ade9
log level in argparse
abe-winter Jun 12, 2024
b657872
test EasyResource
abe-winter Jun 13, 2024
76b9655
rename + test parse_module_args
abe-winter Jun 13, 2024
590ee13
meh we trust argparse
abe-winter Jun 13, 2024
0507d12
docstrings, pyright, lint
abe-winter Jun 13, 2024
4e19ecf
private methods
abe-winter Jun 13, 2024
bbd8c5b
hasattr check in run_with_models
abe-winter Jun 13, 2024
469c92f
usage in run_from_registry
abe-winter Jun 13, 2024
7b31548
remove todo comment
abe-winter Jun 13, 2024
6cbe3e1
remove remaining todo comment
abe-winter Jun 13, 2024
711bb71
use argparse everywhere
abe-winter Jun 14, 2024
ac9fc40
Merge branch 'main' into easy-resource
abe-winter Jun 14, 2024
3347b53
clarify comment
abe-winter Jun 14, 2024
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
Prev Previous commit
Next Next commit
hasattr check in run_with_models
  • Loading branch information
@abe-winter
abe-winter committed Jun 13, 2024
commit bbd8c5ba401c1f8e7f75e56ab754a2a829ce0ede
3 changes: 2 additions & 1 deletion src/viam/module/module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ async def run_with_models(cls, *models: ResourceBase):
args = _parse_module_args()
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?
if not hasattr(model, 'MODEL'):
raise TypeError(f"missing MODEL field on {model}. Resource implementations must define MODEL")
module.add_model_from_registry(model.SUBTYPE, model.MODEL) # pyright: ignore [reportAttributeAccessIssue]
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@abe-winter abe-winter Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

await module.start()

Expand Down