feat(plugins): allow multiple download or auth plugins for a single provider #1704
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t adaptations and fixes
…daptions and fixes
|
Code Coverage (Ubuntu)Diff against developResults for commit: 3d76b87 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
|
Code Coverage (Windows)Diff against developResults for commit: 3d76b87 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
jlahovnik
force-pushed
the
1617-allow-multiple-download-or-auth-plugins-for-a-single-provider
branch
from
e266451 to
3d76b87
Compare
sbrunato
requested changes
Jul 3, 2025
| :param auth_conf: Authentication plugin configuration | ||
| :returns: True if credentials are set, else False | ||
| """ | ||
| return any( |
There was a problem hiding this comment.
move credentials_in_auth to PluginConfig.has_credentials ?
| """Authenticate""" | ||
| raise NotImplementedError | ||
|
|
||
| def get_required_credentials(self) -> dict[str, str]: |
There was a problem hiding this comment.
Suggested change
| def get_required_credentials(self) -> dict[str, str]: | |
| def get_credentials(self) -> dict[str, str]: |
Comment on lines
+47
to
+49
| """checks if only a subset of the credentials is required for the plugin object | ||
| and returns this subset; returns all credentials if not required credentials are given | ||
| :returns: dict of credentials |
There was a problem hiding this comment.
Suggested change
| """checks if only a subset of the credentials is required for the plugin object | |
| and returns this subset; returns all credentials if not required credentials are given | |
| :returns: dict of credentials | |
| """Get plugin configuration credentials. | |
| Checks if only a subset of the credentials is required for the plugin object | |
| and returns this subset. | |
| Returns all credentials if no required credentials are given. | |
| :returns: dict of credentials |
| matching_conf: Optional[PluginConfig], | ||
| ) -> bool: | ||
| """ | ||
| checks if the given plugin config is matching the matching config or url |
There was a problem hiding this comment.
Suggested change
| checks if the given plugin config is matching the matching config or url | |
| Checks if the given plugin config is matching the matching config or url | |
| logger = logging.getLogger("eodag.plugins.manager") | ||
|
|
||
|
|
||
| def _is_plugin_matching( |
There was a problem hiding this comment.
move to PluginConfig.is_matching ?
| }, | ||
| download: dict[str, Any] = {"type": "HTTPDownload", "auth_error_code": 401}, | ||
| download: list[dict[str, Any]] = [], | ||
| **kwargs: dict[str, Any], |
There was a problem hiding this comment.
please replace **kwargs with auth: Optional[list[dict[str, Any]]] = None and api: Optional[dict[str, Any]] = None
Comment on lines
+564
to
+571
| logger.info( | ||
| "%s: provider needing auth for search has been pruned because " | ||
| "no credentials could be found", | ||
| provider, | ||
| ) | ||
| self._pruned_providers_config[ | ||
| provider | ||
| ] = self.providers_config.pop(provider) |
There was a problem hiding this comment.
Suggested change
| logger.info( | |
| "%s: provider needing auth for search has been pruned because " | |
| "no credentials could be found", | |
| provider, | |
| ) | |
| self._pruned_providers_config[ | |
| provider | |
| ] = self.providers_config.pop(provider) | |
| self._pruned_providers_config[ | |
| provider | |
| ] = self.providers_config.pop(provider) | |
| logger.info( | |
| "%s: provider needing auth for search has been pruned because " | |
| "no credentials could be found", | |
| provider, | |
| ) |
| kwargs["auth"] = auth | ||
| else: | ||
| logger.debug( | ||
| f"No authentication plugin for {provider} for product types discovery found" |
There was a problem hiding this comment.
Suggested change
| f"No authentication plugin for {provider} for product types discovery found" | |
| "No authentication plugin for %s for product types discovery found", provider |
| def get_auth( | ||
| self, | ||
| provider: str, | ||
| required_for: str, |
There was a problem hiding this comment.
Suggested change
| required_for: str, | |
| required_for: Literal["search", "download"], |
Comment on lines
+87
to
+90
| returns the url used to find the download and auth plugin for a product | ||
| (currently there is only one matching_url per product, where per asset | ||
| downloaders are implmented, this will change) | ||
| :param eo_product: product for with the matching url shall be found |
There was a problem hiding this comment.
Suggested change
| returns the url used to find the download and auth plugin for a product | |
| (currently there is only one matching_url per product, where per asset | |
| downloaders are implmented, this will change) | |
| :param eo_product: product for with the matching url shall be found | |
| returns the url used to find the download and auth plugin for a product | |
| (currently there is only one matching_url per product, where per asset | |
| downloaders are implmented, this will change) | |
| TODO: adapt method to asset once downloadLink is handled through an asset | |
| :param eo_product: product for with the matching url shall be found |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of a single download or authentication plugin, now a list of plugins is defined for each provider:
search_authanddownload_authhave been removed, both are now given in a list and the parameterrequired_for(list[str]) is used to define if an authentication plugin is used for search, download or both. This also replaces the attributeneed_authof the search plugin. The parameterrequired_credentialsis used to define which credentials are used for which plugin in case of multiple plugins. If it is not given, it is assumed that all available credentials fromeodag.ymlare required.matching_urlandmatching_confhave been replaced by a dictionarymatch. The parameterhrefin the dictionary corresponds tomatching_url, the remaining parameters tomatching_conf.