You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vincbeck (via GitHub)" <gi...@apache.org> on 2024/10/09 15:01:14 UTC
[PR] Change directory used by simple auth manager to store generated passwords [airflow]
vincbeck opened a new pull request, #42860:
URL: https://github.com/apache/airflow/pull/42860
See [discussion](https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1728484901687309?thread_ts=1728483230.037469&cid=C06K9Q5G2UA)
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #42860:
URL: https://github.com/apache/airflow/pull/42860#issuecomment-2403404780
> Maybe we can have a non-specific env variable and define it in BREEZE instead?
>
> say `AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY` ? Then we will just have to set it in `entrypoint_ci.sh`.
Yeah, I like that
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795995567
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
But now I remember this [comment](https://github.com/apache/airflow/pull/42004#discussion_r1750302628). Should we still consider `AIRFLOW_HOME` then? Only when Airflow is installed as package I guess (as you mentioned @potiuk )
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1793820597
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +80,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @cached_property
+ def generated_password_file(self) -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("BREEZE", False):
+ return Path(f"/files/{filename}")
Review Comment:
It feels weird to have Breeze-specific logic in the code
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795820412
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I think we should handle (similarly as with Providers Manager) - two cases, one where airflow (and providers in case of provider's manager) are installed from sources, and another when Airflow is installed as package. In the second case I think good default is somewhere in AIRFLOW_HOME or "HOME/airflow" - this is for example where we create sqlite database when we use sqlite as 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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #42860:
URL: https://github.com/apache/airflow/pull/42860#issuecomment-2405624694
I just did as you suggested :)
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1796013833
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I changed the logic, it is quite simple now:
- if `AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY` is defined, the passwords are saved there. This is the case when run with Breeze (I set it in `entrypoint_ci.sh`)
- If not defined, save the file in `AIRFLOW_HOME`, unless I misunderstood something, I dont think we should make a difference whether Airflow is installed with sources or as a package. In both cases, saving the passwords in `AIRFLOW_HOME` is fine
WDYT?
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795817964
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I think we cannot use this AIRFLOW_ROOT_PATH. The `hatch_build.py` where it is imported from is not available when airflow is installed as package - it's only available from sources. So we should have a bit "saner" default. I thi
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #42860:
URL: https://github.com/apache/airflow/pull/42860
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1793896803
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +80,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @cached_property
+ def generated_password_file(self) -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("BREEZE", False):
+ return Path(f"/files/{filename}")
Review Comment:
Agree but I could not find a better way to do it. The `files` directory is basically in two different locations depending on whether you run Airflow with Breeze
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795817964
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I think we cannot use this AIRFLOW_ROOT_PATH. The `hatch_build.py` where it is imported from is not available when airflow is installed as package - it's only available from sources. So we should have a bit "saner" default.
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1796029380
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +78,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> str:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if directory := os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return os.path.join(directory, filename)
+ else:
+ return os.path.join(AIRFLOW_HOME, filename)
Review Comment:
Shortcut? :-D
```suggestion
return os.path.join(
os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY", AIRFLOW_HOME),
"simple_auth_manager_passwords.json.generated",
)
```
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795854578
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I meant with no branches, use the same path regardless of the environment. I am just trying to make it simple, I feel like it is getting overly complex for something that should not be
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #42860:
URL: https://github.com/apache/airflow/pull/42860#issuecomment-2403399704
Maybe we can have a non-specific env variable and define it in BREEZE instead?
say `AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY` ? Then we will just have to set it in `entrypoint_ci.sh`.
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795998168
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I am trying to wrap my head around all this logic .. Is there any code in place I can take inspiration from? I dont quite know how to figure if Airflow is installed from sources or as a package
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795995567
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
But now I remember this [comment](https://github.com/apache/airflow/pull/42004#discussion_r1750302628). Should we still consider `AIRFLOW_HOME` then?
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1796042850
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +78,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> str:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if directory := os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return os.path.join(directory, filename)
+ else:
+ return os.path.join(AIRFLOW_HOME, filename)
Review Comment:
Way better!
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #42860:
URL: https://github.com/apache/airflow/pull/42860#issuecomment-2403045985
> Looks good for me.
>
> Would be "cool" if not created that upon start via Breeze the default admin/admin is created by Breeze so that after clean checkout it runs out-of-the-box... but not required in this PR...
A user is created if no user is defined and generate the password for you. The file where the password is saved is now in `/files` so that the password does not change every-time you restart your env. You can also modify the password directly in this file and set it to `admin` if you want
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795842866
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I can see that the standalone command uses `password_path = os.path.join(AIRFLOW_HOME, "standalone_admin_password.txt")` to store the password. Should not we do the same? Like: `password_path = os.path.join(AIRFLOW_HOME, "simple_auth_manager_passwords.json.generated")`?
--
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
Re: [PR] Change directory used by simple auth manager to store generated passwords [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #42860:
URL: https://github.com/apache/airflow/pull/42860#discussion_r1795842866
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -78,20 +79,23 @@ class SimpleAuthManager(BaseAuthManager):
:param appbuilder: the flask app builder
"""
- # File that contains the generated passwords
- GENERATED_PASSWORDS_FILE = (
- AIRFLOW_ROOT_PATH / "generated" / "simple_auth_manager_passwords.json.generated"
- )
-
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ @staticmethod
+ def get_generated_password_file() -> Path:
+ filename = "simple_auth_manager_passwords.json.generated"
+ if os.getenv("AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY"):
+ return Path(f"{os.getenv('AIRFLOW_AUTH_MANAGER_CREDENTIAL_DIRECTORY')}/{filename}")
+ else:
+ return AIRFLOW_ROOT_PATH / "files" / filename
Review Comment:
I can see that the standalone command uses `password_path = os.path.join(AIRFLOW_HOME, "standalone_admin_password.txt")` to store the password. Should not we do the same? Like: password_path = os.path.join(AIRFLOW_HOME, "simple_auth_manager_passwords.json.generated")?
--
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