Skip to content

[ASIM] - Change PluginInfo and add some type annotation#145

Merged
BeneBr merged 2 commits intomasterfrom
fb-ASIM-6223-update-metadata
Jun 9, 2025
Merged

[ASIM] - Change PluginInfo and add some type annotation#145
BeneBr merged 2 commits intomasterfrom
fb-ASIM-6223-update-metadata

Conversation

@BeneBr
Copy link
Copy Markdown
Contributor

@BeneBr BeneBr commented Jun 4, 2025

Add some new property requirement which includes the sdk_version, python_version for plugins and add some type annotations.

@BeneBr BeneBr force-pushed the fb-ASIM-6223-update-metadata branch from 87e3ae0 to 873477a Compare June 5, 2025 00:31
Copy link
Copy Markdown
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

However, we should not add "sdk_version" explicitly like this -- SDK is something used by ALFAsim but is not necessarily true for all users of hookman.

We should add a way to add generic requirements, without naming them, for example I can pass {"a": ">=1.2.0", "b": ">=3.0.0"}.

While python_version makes sense, I think it is simpler to just work with a generic requirements: dict[str, str] instead.

For alfasim, we will use hookman passing the requirements explicitly, for example: {"alfasim_sdk": ">=1.0", "python": ">=3.10,<3.11"}.

Comment thread hookman/__main__.py Outdated
@BeneBr BeneBr force-pushed the fb-ASIM-6223-update-metadata branch 4 times, most recently from 6d65a3a to 351db7c Compare June 5, 2025 17:27
@BeneBr BeneBr force-pushed the fb-ASIM-6223-update-metadata branch 3 times, most recently from a149acf to a3f397b Compare June 7, 2025 18:12
Update PluginInfo to accept a new optional requirements dict.
@BeneBr BeneBr force-pushed the fb-ASIM-6223-update-metadata branch from 318932c to 9f06ad5 Compare June 7, 2025 18:13
@BeneBr BeneBr requested a review from nicoddemus June 7, 2025 18:15
@BeneBr BeneBr merged commit 4461816 into master Jun 9, 2025
7 checks passed
@BeneBr BeneBr deleted the fb-ASIM-6223-update-metadata branch June 9, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants