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

(SIMP-10376) Fix simpkv plugin API issues #62

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

lnemsick-simp
Copy link
Contributor

@lnemsick-simp lnemsick-simp commented Jul 16, 2021

Fix two simpkv plugin API issues found when developing the LDAP plugin

  • Fixed the mechanism a plugin uses to advertise its type.
    • Plugin type is now determined from its filename.
    • Previous mechanism did not work when when multiple plugins were used.
      It erroneously used a class method type, which for each anonymous
      plugin class, polluted the Class object namespace, instead of being
      associated with its anonymous class.
  • Changed the plugin configuration API
    • Configuration has been split out into its own method, instead of being
      done in the plugin constructor.
    • This minimal change simplifies unit testing of configuration of complex
      plugins.

SIMP-10376 #close

Fix two simpkv plugin API issues found when developing the LDAP plugin

* Fixed the mechanism a plugin uses to advertise its type.
  - Plugin type is now determined from its filename.
  - Previous mechanism did not work when when multiple plugins were used.
    It erroneously used a class method `type`, which for each anonymous
    plugin class, polluted the Class object namespace, instead of being
    associated with its anonymous class.
* Changed the plugin configuration API
  - Configuration has been split out into its own method, instead of being
    done in the plugin constructor.
  - This minimal change simplifies unit testing of configuration of complex
    plugins.

SIMP-10376 #close
@trevor-vaughan trevor-vaughan merged commit 6288b02 into simp:master Jul 27, 2021
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