Skip to content
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.

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

wheel: initial effort to allow dynamic linking of plugins #88

Closed
wants to merge 12 commits into from

Conversation

tstamler
Copy link
Contributor

@tstamler tstamler commented Mar 25, 2025

This PR removes the need to statically link in plugins to our python module by setting an environment variable in our module init.py. Also removes an old dependency.


__all__ = [nixl]

plugin_dir = nixl.__file__[:-16] + ".nixl.mesonpy.libs/plugins/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the part that gives me anxiety, basically this will find the init.py file. So I'm removing the "nixl/init.py" from the end and then adding ".nixl.mesonpy.libs/plugins/" but I'm not sure if that path will be consistent across systems. open to alternatives

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added an extra check for a different path, might be an ok option

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt you check if NIXL_PLUGIN_DIR is not set and then only do this? You may be overwriting a path set by the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, I've updated it

Add explicit typing for python API parameters (where appropriate).

Remove workarounds to enable fully automatic backend selection for
python users.

Signed-off-by: Timothy Stamler <[email protected]>
Address comments from PR #86:

- Update types
- Add return values to API functions
- Add backend list option to appropriate functions

Signed-off-by: Timothy Stamler <[email protected]>
Signed-off-by: Timothy Stamler <[email protected]>
@tstamler
Copy link
Contributor Author

seems good in testing so far, will test with test.pypi before giving to release team

@tstamler tstamler changed the base branch from main to tstamler/python_api_types March 26, 2025 19:56
@tstamler tstamler force-pushed the tstamler/python_dynamic_plugins branch from 8b055dd to a4d3ee1 Compare March 26, 2025 19:56
@tstamler tstamler marked this pull request as ready for review March 26, 2025 19:56
@@ -23,6 +25,10 @@
if __name__ == "__main__":
buf_size = 256
# Allocate memory and register with NIXL

print("Plugin DIR")
Copy link
Contributor

@aranadive aranadive Mar 26, 2025

Choose a reason for hiding this comment

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

Using NIXL Plugins from:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Base automatically changed from tstamler/python_api_types to main March 26, 2025 21:06
@tstamler tstamler force-pushed the tstamler/python_dynamic_plugins branch from c4ebbcd to c71c0c4 Compare March 26, 2025 21:20
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