You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/03 10:32:19 UTC
[GitHub] [airflow] raphaelauv opened a new pull request #19332: Add a skip parameter to Variable to skip the secret backend
raphaelauv opened a new pull request #19332:
URL: https://github.com/apache/airflow/pull/19332
close #19251
the new parameter will skip ~~all external~~ the secret backend for variables
so it will not skip the [env variables](https://airflow.apache.org/docs/apache-airflow/stable/howto/variable.html#storing-variables-in-environment-variables) set by `AIRFLOW_VAR_FOO`
and the variables inside the airflow db
and (if setup) a LocalFilesystemBackend
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958954213
My problem with this solution is making the decision something that each dag author has to do on a case-by-base basis, rather than a "global" setting that applies to a given variable (name) always.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
( that's why originally I come up with the name `skip_external_backends` )
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb closed pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #19332:
URL: https://github.com/apache/airflow/pull/19332
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a LocalFilesystemBackend setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
( that's why originally I come up with the name `skip_external_backends` )
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb closed pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #19332:
URL: https://github.com/apache/airflow/pull/19332
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958832770
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958950868
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741681176
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backends"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backend"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
Also plural suggests that we can have more than one "external/secret" backend configured - which we don't have (and will not likely have), so I suggest to use singular form.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958863995
Not really. This one is for selective disabling of secret backends only for some cases which user chooses consciously to skip secrets querying (knowing that the data is not secret). This is an optimisation of cost mostly (many secret backends are paid by access)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958972943
Proposal: Let's continue the discussion in #19251
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958950868
The option is false by default. That mean only when user is using Variable.get() himself he can custom the behavior. So I don't see any hidden behavior.
Yes extending the custom_backend is an option , I still think that for the majority of the calls to the variables are for the airflow DB and not the custom secret backend.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958832770
Not needed, possible via secrets backend configuration https://github.com/apache/airflow/blob/ae044884d1dacce8dbf47c618f543b58f9ff101f/airflow/providers/google/cloud/secrets/secret_manager.py#L60-L62
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a LocalFilesystemBackend setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
( that's why originally I come up with the name `skip_external_backends` )
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958950868
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958863995
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958832770
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741681176
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backends"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backend"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
Also plural suggests that we can have more than one "external/secret" backend configured - which we don't have (and will not likely have), so I suggest to use singular form.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741681176
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backends"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a LocalFilesystemBackend setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958959534
Yes it's perflectly true that it's going to be boring to setup almost all the variables to
`Variable.get("toto",skip_secret_backend=True)`
a better option would be a prefix-pattern-rule for variables to lookup
so a new common option to `backend_kwargs`
`"only_look_up_prefix_variables" : "secret_"`
for Variable.get("secret_toto") the secret_backend would be call
and Variable.get("toto") the secret_backend would directly return None and the other backend would then be check
This would be great and explicit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958832770
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741681176
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backend"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb closed pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #19332:
URL: https://github.com/apache/airflow/pull/19332
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958930776
@potiuk That strikes me as a very-hard to reason about feature -- depending on _how_ the DAG accesses the variable means that you get different behaviour. If I do it one way the it will load the secret from the secrets store, but if I do it another way it will use the value in the env/DB.
This makes the runtime behaviour hard to reason about and makes the system as a whole hard to reason about.
This is a -1/veto from me, sorry.
If you still want this sort of feature @raphaelauv then I would suggest you write a custom secrets backend (subclassing the existing GCS backend) and make a decision based on the name/prefix of the variable. That way the source of a variable doesn't depend on _how_ it is accessed in different DAGS.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv edited a comment on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv edited a comment on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958959534
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741727692
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a LocalFilesystemBackend setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
that's why originally I come up with the name `skip_external_backends`
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
the "problem" is that the `LocalFilesystemBackend` need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html
but we don't want skip it cause it's not an "external" backend
so `skip_secret_backend` will not be very clear for people with a `LocalFilesystemBackend` setup
( that's why originally I come up with the name `skip_external_backends` )
what should we do ?
the documentation of the arg should precise the 3 not skipped backends ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb closed pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #19332:
URL: https://github.com/apache/airflow/pull/19332
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv edited a comment on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv edited a comment on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958959534
Yes it's perflectly true that it's going to be boring to setup almost all the variables to
`Variable.get("toto",skip_secret_backend=True)`
a better option would be a prefix-pattern-rule for variables to lookup
so a new common option to `backend_kwargs`
`"only_look_up_prefix_variables" : "secret_"`
for `Variable.get("secret_toto")` the secret_backend would be call
and `Variable.get("toto")` the secret_backend would directly return None and the other backend would then be check
This would be great and explicit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741682109
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
Also plural suggests that we can have more than one "external/secret" backend configured - which we don't have (and will not likely have), so I suggest to use singular form.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19332: Add a skip parameter to Variable to skip the secret backend(s)
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#discussion_r741681176
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backends"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
should we rename it to "skip_secret_backend"? I think this would be more consistent with the currently used naming
https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html
For example we can have local FilesystemBackend (which is not `external`). Also it would indicate better the intentions.
What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.
##########
File path: airflow/models/variable.py
##########
@@ -124,15 +125,18 @@ def get(
key: str,
default_var: Any = __NO_DEFAULT_SENTINEL,
deserialize_json: bool = False,
+ skip_external_backends: bool = False,
Review comment:
Also plural suggests that we can have more than one "external/secret" backend configured - which we don't have (and will not likely have), so I suggest to use singular form.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958863995
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raphaelauv edited a comment on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
raphaelauv edited a comment on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958959534
Yes it's perflectly true that it's going to be boring to setup almost all the variables to
`Variable.get("toto",skip_secret_backend=True)`
a better option would be a prefix-pattern-rule for variables to lookup
so a new common option to `backend_kwargs`
`"only_look_up_prefix_variables" : "secret_"`
for `Variable.get("secret_toto")` the secret_backend would be call
and `Variable.get("toto")` the secret_backend would directly return None and the other backend would then be check
This would be great and explicit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19332: Add a skip parameter to Variable to skip the secret backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19332:
URL: https://github.com/apache/airflow/pull/19332#issuecomment-958863995
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org