Copilot commented on code in PR #64879:
URL: https://github.com/apache/airflow/pull/64879#discussion_r3066472557
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -300,9 +300,13 @@ def _fetch_bare_repo(self):
reraise=True,
)
def _fetch_submodules(self) -> None:
- self._log.info("Initializing and updating submodules",
repo_path=self.repo_path)
- self.repo.git.submodule("sync", "--recursive")
- self.repo.git.submodule("update", "--init", "--recursive", "--jobs",
"1")
+ cm = nullcontext()
+ if self.hook and (cmd := self.hook.env.get("GIT_SSH_COMMAND")):
+ cm = self.repo.git.custom_environment(GIT_SSH_COMMAND=cmd)
+ with cm:
Review Comment:
The name `cm` is very generic for a value that controls an environment
override. Consider renaming it to something more descriptive (e.g., `env_cm` /
`ssh_env_cm`) to make the intent clearer when reading and debugging this method.
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -300,9 +300,13 @@ def _fetch_bare_repo(self):
reraise=True,
)
def _fetch_submodules(self) -> None:
- self._log.info("Initializing and updating submodules",
repo_path=self.repo_path)
- self.repo.git.submodule("sync", "--recursive")
- self.repo.git.submodule("update", "--init", "--recursive", "--jobs",
"1")
+ cm = nullcontext()
+ if self.hook and (cmd := self.hook.env.get("GIT_SSH_COMMAND")):
+ cm = self.repo.git.custom_environment(GIT_SSH_COMMAND=cmd)
+ with cm:
+ self._log.info("Initializing and updating submodules",
repo_path=self.repo_path)
+ self.repo.git.submodule("sync", "--recursive")
+ self.repo.git.submodule("update", "--init", "--recursive",
"--jobs", "1")
Review Comment:
This change introduces new behavior (wrapping submodule sync/update with
`custom_environment` when `GIT_SSH_COMMAND` is present). Add/extend a unit test
to assert `custom_environment(GIT_SSH_COMMAND=...)` is invoked (and that
submodule commands run within that context) when `hook.env` provides
`GIT_SSH_COMMAND`.
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -300,9 +300,13 @@ def _fetch_bare_repo(self):
reraise=True,
)
def _fetch_submodules(self) -> None:
- self._log.info("Initializing and updating submodules",
repo_path=self.repo_path)
- self.repo.git.submodule("sync", "--recursive")
- self.repo.git.submodule("update", "--init", "--recursive", "--jobs",
"1")
+ cm = nullcontext()
Review Comment:
`nullcontext` must be in scope for this to run. If it isn’t already imported
in this module, add `from contextlib import nullcontext` (or reference it as
`contextlib.nullcontext`) to avoid a `NameError` at runtime.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
