You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jx2lee (via GitHub)" <gi...@apache.org> on 2025/01/28 14:56:04 UTC
[PR] Enable to add inline ssh key in GitHook [airflow]
jx2lee opened a new pull request, #46181:
URL: https://github.com/apache/airflow/pull/46181
<!--
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/
-->
closes: #45979
All tests in `test_dag_bundles` will pass after merging #46179
<!-- 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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1937441076
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -84,6 +94,13 @@ def _process_git_auth_url(self):
elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"):
self.repo_url = os.path.expanduser(self.repo_url)
+ def _setup_inline_key(self):
+ with tempfile.NamedTemporaryFile(mode="w", delete=False) as tmp_keyfile:
Review Comment:
I've completed the refactoring based on your comments.
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1941041959
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
I'm not sure fully applied your comments. Updated the code first, please review the new code and provide your comment again!
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1942036976
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
Close! Just a couple polishing comments, but overall lgtm.
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1932562511
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -60,6 +61,7 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"extra": json.dumps(
{
"key_file": "optional/path/to/keyfile",
+ "private_key": "optional/private_key",
Review Comment:
nit: maybe say "optional inline private key", just don't make it look like a path basically.
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -84,6 +94,13 @@ def _process_git_auth_url(self):
elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"):
self.repo_url = os.path.expanduser(self.repo_url)
+ def _setup_inline_key(self):
+ with tempfile.NamedTemporaryFile(mode="w", delete=False) as tmp_keyfile:
Review Comment:
We might want to refactor things a bit.
- We shouldn't leave this fine around imo, so let's do a context manager so we can clean up.
- This likely means the way we do env likely needs to be adjusted a bit too.
- This also means we won't have to write the pk out when creating a bundle just to do the view_url (e.g. on the webserver).
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -160,6 +184,16 @@ def test_correct_repo_urls(self, conn_id, expected_repo_url):
hook = GitHook(git_conn_id=conn_id)
assert hook.repo_url == expected_repo_url
+ def test_given_both_private_key_and_key_file(self):
+ with pytest.raises(
+ AirflowException, match="Both 'key_file' and 'private_key' cannot be provided at the same time"
+ ):
+ GitHook(git_conn_id=CONN_BOTH_PATH_INLINE)
+
+ def test_only_inline_connection_has_tmp_keyfile(self):
Review Comment:
We should beef up our coverage on this path too. "how" depends how we refactor I think, but reach out if you need ideas :)
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -70,10 +72,18 @@ def __init__(self, git_conn_id="git_default", *args, **kwargs):
connection = self.get_connection(git_conn_id)
self.repo_url = connection.host
self.auth_token = connection.password
+ self.private_key = connection.extra_dejson.get("private_key")
self.key_file = connection.extra_dejson.get("key_file")
self.env: dict[str, str] = {}
+
+ if self.key_file and self.private_key:
+ raise AirflowException("Both 'key_file' and 'private_key' cannot be provided at the same time")
+ if self.private_key:
+ self._setup_inline_key()
if self.key_file:
Review Comment:
```suggestion
if self.key_file or self.private_key:
```
right?
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1940972396
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
Review Comment:
Oops, added it!
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1942041852
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
😱 As i kept changing code, lot of mistakes. I'll fix, soon!
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1942696792
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
@jedcunningham Fixed Done! and test code, too. Review again, please. 🙏
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1940374834
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
Review Comment:
You'll also need to do this for refresh.
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
It might make sense to set `env` in the context manager for both the key_file and private_key paths. Right now it's a bit spread out and inconsistent. You can also rename the context manager to be a bit more general then too.
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -144,6 +147,17 @@ def setup_class(cls) -> None:
conn_id=CONN_ONLY_PATH,
host="path/to/repo",
conn_type="git",
+ extra={"key_file": "path/to/key"},
Review Comment:
Adding this sorta defeats the purpose - you don't need a key for a local path :)
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -160,6 +184,16 @@ def test_correct_repo_urls(self, conn_id, expected_repo_url):
hook = GitHook(git_conn_id=conn_id)
assert hook.repo_url == expected_repo_url
+ def test_given_both_private_key_and_key_file(self):
+ with pytest.raises(
+ AirflowException, match="Both 'key_file' and 'private_key' cannot be provided at the same time"
+ ):
+ GitHook(git_conn_id=CONN_BOTH_PATH_INLINE)
+
+ def test_only_inline_connection_has_tmp_keyfile(self):
Review Comment:
Looks good now I think.
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1941008188
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
- Renamed context manager
- Modified context manager, with ssh_command create another command by key_file or private_key
I'm not sure fully understood your comments.
Updated the code first, please review the new code and provide your comment again!
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1942036081
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -70,15 +73,24 @@ def __init__(self, git_conn_id="git_default", *args, **kwargs):
connection = self.get_connection(git_conn_id)
self.repo_url = connection.host
self.auth_token = connection.password
+ self.private_key = connection.extra_dejson.get("private_key")
self.key_file = connection.extra_dejson.get("key_file")
- strict_host_key_checking = connection.extra_dejson.get("strict_host_key_checking", "no")
+ self.strict_host_key_checking = connection.extra_dejson.get("strict_host_key_checking", "no")
self.env: dict[str, str] = {}
+
+ if self.key_file and self.private_key:
+ raise AirflowException("Both 'key_file' and 'private_key' cannot be provided at the same time")
if self.key_file:
Review Comment:
```suggestion
```
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -70,15 +73,24 @@ def __init__(self, git_conn_id="git_default", *args, **kwargs):
connection = self.get_connection(git_conn_id)
self.repo_url = connection.host
self.auth_token = connection.password
+ self.private_key = connection.extra_dejson.get("private_key")
self.key_file = connection.extra_dejson.get("key_file")
- strict_host_key_checking = connection.extra_dejson.get("strict_host_key_checking", "no")
+ self.strict_host_key_checking = connection.extra_dejson.get("strict_host_key_checking", "no")
self.env: dict[str, str] = {}
+
+ if self.key_file and self.private_key:
+ raise AirflowException("Both 'key_file' and 'private_key' cannot be provided at the same time")
if self.key_file:
- self.env["GIT_SSH_COMMAND"] = (
- f"ssh -i {self.key_file} -o IdentitiesOnly=yes -o StrictHostKeyChecking={strict_host_key_checking}"
- )
+ self.env["GIT_SSH_COMMAND"] = self._build_ssh_command(self.key_file)
Review Comment:
```suggestion
```
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -87,6 +99,25 @@ def _process_git_auth_url(self):
elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"):
self.repo_url = os.path.expanduser(self.repo_url)
+ def set_git_env(self, key: str) -> dict[str, str]:
+ if self.key_file:
+ return self.env
+ self.env["GIT_SSH_COMMAND"] = self._build_ssh_command(key)
+ return self.env
+
+ @contextlib.contextmanager
+ def configure_hook_env(self):
+ if self.private_key:
+ with tempfile.NamedTemporaryFile(mode="w", delete=True) as tmp_keyfile:
+ tmp_keyfile.write(self.private_key)
+ tmp_keyfile.flush()
+ os.chmod(tmp_keyfile.name, 0o600)
+ self.set_git_env(tmp_keyfile.name)
+ yield
+ else:
+ self.set_git_env(self.private_key)
Review Comment:
```suggestion
self.set_git_env(self.key_file)
```
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -128,8 +159,11 @@ def __init__(
self.log.warning("Could not create GitHook for connection %s : %s", self.git_conn_id, e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.configure_hook_env() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
```suggestion
with self.hook.configure_hook_env():
```
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -87,6 +99,25 @@ def _process_git_auth_url(self):
elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"):
self.repo_url = os.path.expanduser(self.repo_url)
+ def set_git_env(self, key: str) -> dict[str, str]:
+ if self.key_file:
+ return self.env
+ self.env["GIT_SSH_COMMAND"] = self._build_ssh_command(key)
+ return self.env
Review Comment:
```suggestion
def set_git_env(self, key: str) -> None:
self.env["GIT_SSH_COMMAND"] = self._build_ssh_command(key)
```
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1941008188
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
- Renamed context manager
- Modified context manager, with ssh_command create another command by key_file or private_key
I'm not sure fully understood your comments.
Updated the code first, please review the new code and provide your comment again!
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1941041959
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -127,8 +156,11 @@ def __init__(
self.log.error("Error creating GitHook: %s", e)
def _initialize(self):
- self._clone_bare_repo_if_required()
- self._ensure_version_in_bare_repo()
+ with self.hook.setup_inline_key() as tmp_keyfile:
+ self.hook.env = self.hook.set_git_env(tmp_keyfile)
Review Comment:
I'm not sure fully applied your comments. Updated the code first, please review the new code and provide your comment again! (in new thread)
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1932581509
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -70,10 +72,18 @@ def __init__(self, git_conn_id="git_default", *args, **kwargs):
connection = self.get_connection(git_conn_id)
self.repo_url = connection.host
self.auth_token = connection.password
+ self.private_key = connection.extra_dejson.get("private_key")
self.key_file = connection.extra_dejson.get("key_file")
self.env: dict[str, str] = {}
+
+ if self.key_file and self.private_key:
+ raise AirflowException("Both 'key_file' and 'private_key' cannot be provided at the same time")
+ if self.private_key:
+ self._setup_inline_key()
if self.key_file:
Review Comment:
Oh, I fixed it! Thanks.
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1937443493
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -160,6 +184,16 @@ def test_correct_repo_urls(self, conn_id, expected_repo_url):
hook = GitHook(git_conn_id=conn_id)
assert hook.repo_url == expected_repo_url
+ def test_given_both_private_key_and_key_file(self):
+ with pytest.raises(
+ AirflowException, match="Both 'key_file' and 'private_key' cannot be provided at the same time"
+ ):
+ GitHook(git_conn_id=CONN_BOTH_PATH_INLINE)
+
+ def test_only_inline_connection_has_tmp_keyfile(self):
Review Comment:
Hmm... private methods were only applied to the context manager, can you suggest test in GitDagBundle?
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1937443493
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -160,6 +184,16 @@ def test_correct_repo_urls(self, conn_id, expected_repo_url):
hook = GitHook(git_conn_id=conn_id)
assert hook.repo_url == expected_repo_url
+ def test_given_both_private_key_and_key_file(self):
+ with pytest.raises(
+ AirflowException, match="Both 'key_file' and 'private_key' cannot be provided at the same time"
+ ):
+ GitHook(git_conn_id=CONN_BOTH_PATH_INLINE)
+
+ def test_only_inline_connection_has_tmp_keyfile(self):
Review Comment:
Umm.. only private method is adjusted context manager, so could you suggest test in GitDagBundle?
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1932582480
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -60,6 +61,7 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"extra": json.dumps(
{
"key_file": "optional/path/to/keyfile",
+ "private_key": "optional/private_key",
Review Comment:
Great, Thanks.
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham merged PR #46181:
URL: https://github.com/apache/airflow/pull/46181
--
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] Enable to add inline ssh key in GitHook [airflow]
Posted by "jx2lee (via GitHub)" <gi...@apache.org>.
jx2lee commented on code in PR #46181:
URL: https://github.com/apache/airflow/pull/46181#discussion_r1940977591
##########
tests/dag_processing/test_dag_bundles.py:
##########
@@ -144,6 +147,17 @@ def setup_class(cls) -> None:
conn_id=CONN_ONLY_PATH,
host="path/to/repo",
conn_type="git",
+ extra={"key_file": "path/to/key"},
Review Comment:
🙏🏽 Yes, Done!
--
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