Skip to content

Conversation

@marquiz
Copy link
Contributor

@marquiz marquiz commented Aug 11, 2025

Use the given name instead of erroring out. The command line flag takes precedence over the NRI_PLUGIN_NAME environment variable.

@marquiz
Copy link
Contributor Author

marquiz commented Aug 11, 2025

Use the given name instead of erroring out. The command line flag takes
precedence over the NRI_PLUGIN_NAME environment variable.

Signed-off-by: Markus Lehtonen <[email protected]>
Copy link
Member

@klihub klihub left a comment

Choose a reason for hiding this comment

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

@marquiz I had a similar one cooking but we can also take this one in instead.

However, IMO it would be better to not allow/do this for 'pre-launched' plugins (optional configuration picking/relaying uses the name to pick configuration for the plugin). The easiest way to achieve that is to do an os.Getenv(api.PluginSocketEnvVar) == "" check here and otherwise either error out, or give a warning/error but not override the name.

@marquiz
Copy link
Contributor Author

marquiz commented Aug 11, 2025

it would be better to not allow/do this for 'pre-launched' plugins (optional configuration picking/relaying uses the name to pick configuration for the plugin). The easiest way to achieve that is to do an os.Getenv(api.PluginSocketEnvVar) == "" check here and otherwise either error out, or give a warning/error but not override the name.

I see the problem. But can we come up with any less kludge-ish way to solve this? What if we just drop the problematic WithPluginName option from the stub? I still remain my comment in #200 that the naming craziness makes no sense to me.

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.

2 participants