JavierLopezT commented on a change in pull request #9008:
URL: https://github.com/apache/airflow/pull/9008#discussion_r466333453
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -60,15 +59,23 @@ class SecretsManagerBackend(BaseSecretsBackend,
LoggingMixin):
def __init__(
self,
- connections_prefix: str = 'airflow/connections',
- variables_prefix: str = 'airflow/variables',
- profile_name: Optional[str] = None,
- sep: str = "/",
+ connections_prefix=None,
+ variables_prefix=None,
Review comment:
Anyway, I don't see that these should be required arguments. In my
current company, we share an AWS account with other departments. As so, we have
to start our secrets with the prefix 'data_', hence in my current situation it
does make sense to have `connections_prefix='data'`. However, in my previous
company, we as BI team had an AWS account for ourselves, and thus we had the
secrets just like _'db_redshift'_, _'api_salesforce'_ and so on, so the None
value made more sense.
Furthermore, I would never expect to have the default variables as
airflow/variables, because I won't ever think at all using the '/' sep in this
case.
I know that what I think or believe is pointless, but I am wondering if
there are more people with similar reasonings as me that the default values
don't fit with their project.
Also, just curiosity (not saying that this is a case in which should be
applied), is there any procedure to implement a change that breaks previous
workflows?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
