-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add plugin interface for external provider registration #1465
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
Conversation
This commit adds a simple and standardized interface for external plugins to register providers and have them work natively off of the qiskit namespace. With this anyone can setup providers to be discoverable and automatically loaded into qiskit-terra if they're installed. It leverages setuptools entry points to define an entry point to basically advertise to terra that a python package provides a backend provider. Terra now leverages the stevedore library (which just provides a useful API ontop of entrypoints and plugins) to discover the providers installed.
86adccb
to
ef062e4
Compare
How does stevedore support prospective qiskit plugins that can't be done with builtin methods, e.g. setuptools entry_points, which it is built upon? |
The stevedore bit is just for discovery of plugins that advertise themselves as such using entry_points. If a plugin doesn't expose the entry point at install time stevedore won't do anything with it (since it won't see it). If a plugin can't use entry_points for some reasons it'll still work like things already do today where you have to manually import the plugin from wherever and access it directly (like in the case of JKU you would use |
My understanding was that setuptools already does discovery of plugins, |
@ewinston yeah, stevedore is just using that under the covers. It provides a nicer api to work with than the raw setuptools. |
So with this patch, the only difference would be that the user writes: Not sure if that's desired. Wouldn't it somehow imply that JKUProvider belongs to qiskit, rather than being clearly a 3rd party installed package? |
@ajavadia Well it's actually from As for some providers using vs not, it's true we can only provide the interface but not force an external provider to use. But, I figure if we clearly document it and the benefits of using it people will start to adopt it, but we can't force anyone to use it. Just like we can't really force anyone to inherit from our abstract provider classes today. |
So if the 3rd party provider just does a directory reorganization and puts their code under |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the doc/providers should go to qiskit/doc
Yeah, I wrote this before the docs migration. I left it in the PR because it helps explain what the patch does. I will remove it from this PR an open up a docs one when we're ready to merge this. |
@mtreinish can you address my comment above? I'm not sure that what a backend developer has to do here is different from just organizing their namespace under |
This actually isn't a solution for third party providers. Having external provider isn't something I would recommend generally, mostly because it is confusing to have an import from qiskit that isn't maintained by a qiskit project. There are also edge cases with namespace packages which we can workaround by maintaining the projects by the same people. I'm not really comfortable encouraging external projects using that mechanism. But even ignoring all of that, simply moving code to This plugin provider makes the namespaces for external code live outside of If you'd like I can write an example patch for JKU or another provider, or I can just write a fake provider plugin example to show how this works. |
Terra change Qiskit/qiskit#1465 adds a plugin interface to allow external providers to have a common interface with terra. It also providers users a consistent place to access external providers. This commit adds the documentation for this new feature.
I moved the docs from here to the qiskit repo in: #190 |
closing this until there's a clear use case for doing this (i.e. there is complaint about 3rd party provider being hard to use under its own namespace). I think there is value in the qisksit namespace being preserved for things we provide. |
Namespace packages are constant source of problems for users. The python packaging ecosystem around splitting packages across namespaces is fragile at the best of times and can often leave a you with an environment that isn't recoverable (especially when mixing install methods). There is also a performance hit whenever there is a piece of the namespace we allow external packages to extend since it requires doing a full python path search which can be slow depending on the backing I/O and the number of paths in sys.path for an environment. This commit starts the process of addressing this by removing the arbitrary namespace hook points and hard coding the element namespace maps via a custom import loader at the root of the namespace. This has 2 advantages it removes the use of namespace packages so the fragility and performance impact are fixed since every element will be renamed to use 'qiskit_*' instead of 'qiskit.*', but it also makes it explicit where we extend the namespace. The previous method allowed any package to extend qiskit.* and qiskit.providers.* with whatever they wanted. We'll need to coordinate updating the elements with this merging, because it is a breaking change for each element (although not for end users). A potential follow on is to add a plugin interface for 3rd party providers like what was proposed in Qiskit#1465 so that we can make external providers externally discoverable without needing to add manual hook points moving forward (this was done for backwards compat with the aqt and honeywell provider). Fixes Qiskit#559
* Remove namespace packaging and hardcode elements Namespace packages are constant source of problems for users. The python packaging ecosystem around splitting packages across namespaces is fragile at the best of times and can often leave a you with an environment that isn't recoverable (especially when mixing install methods). There is also a performance hit whenever there is a piece of the namespace we allow external packages to extend since it requires doing a full python path search which can be slow depending on the backing I/O and the number of paths in sys.path for an environment. This commit starts the process of addressing this by removing the arbitrary namespace hook points and hard coding the element namespace maps via a custom import loader at the root of the namespace. This has 2 advantages it removes the use of namespace packages so the fragility and performance impact are fixed since every element will be renamed to use 'qiskit_*' instead of 'qiskit.*', but it also makes it explicit where we extend the namespace. The previous method allowed any package to extend qiskit.* and qiskit.providers.* with whatever they wanted. We'll need to coordinate updating the elements with this merging, because it is a breaking change for each element (although not for end users). A potential follow on is to add a plugin interface for 3rd party providers like what was proposed in #1465 so that we can make external providers externally discoverable without needing to add manual hook points moving forward (this was done for backwards compat with the aqt and honeywell provider). Fixes #559 * Fix lint * Fix lint * Handle missing ibmq in job watcher * Suppress packaging warnings in docs job * Use find_spec instead of find_module * Fix lint and remove deprecated module_repr method * Add back module_repr, pylint wants it there because of abc
Namespace packages are constant source of problems for users. The python packaging ecosystem around splitting packages across namespaces is fragile at the best of times and can often leave a you with an environment that isn't recoverable (especially when mixing install methods). There is also a performance hit whenever there is a piece of the namespace we allow external packages to extend since it requires doing a full python path search which can be slow depending on the backing I/O and the number of paths in sys.path for an environment. This commit starts the process of addressing this by removing the arbitrary namespace hook points and hard coding the element namespace maps via a custom import loader at the root of the namespace. This has 2 advantages it removes the use of namespace packages so the fragility and performance impact are fixed since every element will be renamed to use 'qiskit_' instead of 'qiskit.', but it also makes it explicit where we extend the namespace. The previous method allowed any package to extend qiskit.* and qiskit.providers.* with whatever they wanted. We'll need to coordinate updating the elements with this merging, because it is a breaking change for each element (although not for end users). A potential follow on is to add a plugin interface for 3rd party providers like what was proposed in Qiskit#1465 so that we can make external providers externally discoverable without needing to add manual hook points moving forward (this was done for backwards compat with the aqt and honeywell provider). This is a second attempt at removing namespace packaging. The first attempt in PR Qiskit#4767 was merged and had to be reverted because there were some circular import error issues that needed to be resolved. Since having this in terra blocks CI for all the qiskit elements a revert was necessary to unblock developement for the entire project while those were resolved.
* Remove namespace packaging and hardcode elements (attempt 2) Namespace packages are constant source of problems for users. The python packaging ecosystem around splitting packages across namespaces is fragile at the best of times and can often leave a you with an environment that isn't recoverable (especially when mixing install methods). There is also a performance hit whenever there is a piece of the namespace we allow external packages to extend since it requires doing a full python path search which can be slow depending on the backing I/O and the number of paths in sys.path for an environment. This commit starts the process of addressing this by removing the arbitrary namespace hook points and hard coding the element namespace maps via a custom import loader at the root of the namespace. This has 2 advantages it removes the use of namespace packages so the fragility and performance impact are fixed since every element will be renamed to use 'qiskit_' instead of 'qiskit.', but it also makes it explicit where we extend the namespace. The previous method allowed any package to extend qiskit.* and qiskit.providers.* with whatever they wanted. We'll need to coordinate updating the elements with this merging, because it is a breaking change for each element (although not for end users). A potential follow on is to add a plugin interface for 3rd party providers like what was proposed in #1465 so that we can make external providers externally discoverable without needing to add manual hook points moving forward (this was done for backwards compat with the aqt and honeywell provider). This is a second attempt at removing namespace packaging. The first attempt in PR #4767 was merged and had to be reverted because there were some circular import error issues that needed to be resolved. Since having this in terra blocks CI for all the qiskit elements a revert was necessary to unblock developement for the entire project while those were resolved. * Try using new aer path for qiskit.Aer alias * Try moving Aer and IBMQ alias to the end * Fix typo * Fix circular import issue * Fix typos * Run black * Remove file encoding * Adjust init with import re-direct * Make qiskit_aer a opportunistic load To ensure that someone with an old version of Aer installed can still access it via the old namespace this changes the meta finder logic to first try qiskit_aer, if it's importable then we build the redirect path first and use that for the legacy qiskit.providers.aer path. If it's not then we just return None and fall back to the other finders in sys.meta_path. To support this the pkgutil hook is added back to qiskit.providers to add the namespace hook for old version of aer using namespace packagingm although not strictly necessary because the implicit support for namespace packages will still likely work we can remove it at a later date. * Deprecate qiskit.Aer entrypoint * Fix lint * Remove unnecessary noqa comments * Revert version.py change and use old aer import for now * Fix typo * Add back pkgutil extension to root init * Make redirect hook more generic * Add comments explaining the various hooks * Make qiskit.Aer a pending deprecation instead of a deprecation * Restrict namespace redirect error to ModuleNotFoundError * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
This commit adds a simple and standardized interface for external
plugins to register providers and have them work natively off of the
qiskit namespace. With this anyone can setup providers to be
discoverable and automatically loaded into qiskit-terra if they're
installed. It leverages setuptools entry points to define an entry point
to basically advertise to terra that a python package provides a
backend provider. Terra now leverages the stevedore library (which just
provides a useful API ontop of entrypoints and plugins) to discover
the providers installed.
Details and comments