You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Pandry (via GitHub)" <gi...@apache.org> on 2026/03/05 14:41:37 UTC
[PR] git dag bundle to use the private key in submodule pull [airflow]
Pandry opened a new pull request, #62938:
URL: https://github.com/apache/airflow/pull/62938
The git dag bundle did not use the private key to
pull submodules.
This lead to issues when working with private submodules. This commit uses the same key used to pull the dag bundle to pull the submodules
I literally copy-pasted the code from `_fetch_bare_repo`
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4476071582
@Pandry A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Pre-commit / static checks**: `CI image checks / Static checks` is failing. Run `prek run --from-ref main --stage pre-commit` locally and fix anything that flags. See the [static-checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
**What to do next:**
- Push a fix for the static-check failure.
No rush — take your time. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Pandry (via GitHub)" <gi...@apache.org>.
Pandry commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4055691587
Oh; I saw the diff but I was not 100% sure it was the newline!
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4179107713
@Pandry This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Build docs**: Failing: CI image checks / Build documentation (--docs-only). Run `breeze build-docs` locally to reproduce. See [Build docs docs](https://github.com/apache/airflow/blob/main/contributing-docs/16_documentation.rst).
- :x: **Other failing CI checks**: Failing: CI image checks / Build documentation (--spellcheck-only), Low dep tests:core / All-core:LowestDeps:14:3.10:API...Serialization. Run `prek run --from-ref main` locally to reproduce. See [static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
> **Note:** Your branch is **867 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
**What to do next:**
- The comment informs you what you need to do.
- Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
- There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
- Maintainers will then proceed with a normal review.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-5149995558
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Pandry (via GitHub)" <gi...@apache.org>.
Pandry commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4179240071
Hi, I just rebased on main, but the build docs link you provided is a 404 (also I did not touch the documentation)
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Pandry (via GitHub)" <gi...@apache.org>.
Pandry commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4529802611
Yeah, sorry, I've been very sloppy with this, got a lot going on and it's not pressing because I added the patch as part of my image pipeline
---
I was sure I tested everything and all tests passed (including the pre-commit) but I might have done that before the commit, so the pre-commit changed the files but I didn't commit them
I'll have a look now and hopefully fix it once and for all!
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4414132343
There are still errors/
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #62938:
URL: https://github.com/apache/airflow/pull/62938
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Pandry (via GitHub)" <gi...@apache.org>.
Pandry commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4198436887
Hi, I didn't hear back from my last comment
I've just rebased agin from main
All the failing tests are not about my commit in particular, so I assume it is fine to perhaps submit again for review?
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4297431461
@Pandry Thanks for your earlier replies — some threads are still marked unresolved and there has been no movement for 20 days. This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Unresolved review comments** (1 thread): please walk through each unresolved review thread. **Even if a suggestion looks incorrect or irrelevant** — and some of them will be, especially any comments left by automated reviewers like GitHub Copilot — it is still the author's responsibility to respond: apply the fix, reply in-thread with a brief explanation of why the suggestion does not apply, or resolve the thread if the feedback is no longer relevant. Leaving threads unaddressed for weeks blocks the PR from moving forward.
**What to do next:**
- Walk through each unresolved review thread and respond as described above.
- Make sure static checks pass locally: `prek run --from-ref main --stage pre-commit`.
- Mark the PR as "Ready for review" when you're done.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4300309845
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item:
Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — **please mark the thread as resolved yourself** by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project.
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62938:
URL: https://github.com/apache/airflow/pull/62938#discussion_r3066479884
##########
providers/git/src/airflow/providers/git/bundles/git.py:
##########
@@ -301,8 +301,12 @@ def _fetch_bare_repo(self):
)
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.repo.git.submodule("sync", "--recursive")
+ self.repo.git.submodule("update", "--init", "--recursive", "--jobs", "1")
Review Comment:
The new submodule fetch behavior relies on setting `GIT_SSH_COMMAND` via `Repo.git.custom_environment(...)`, but the existing unit tests only assert that `submodule sync/update` are invoked and do not verify that the SSH environment is applied. Please add a regression test that sets `mock_githook.return_value.env = {"GIT_SSH_COMMAND": "..."}` and asserts `mock_repo_instance.git.custom_environment` is entered (and that submodule commands run within that context), so private submodule authentication is actually covered.
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "Pandry (via GitHub)" <gi...@apache.org>.
Pandry commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4320256604
Added tests (and ensure they don't pass with the previous commit)
Hopefully that's good now!
--
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] git dag bundle to use the private key in submodule pull [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #62938:
URL: https://github.com/apache/airflow/pull/62938#issuecomment-4529582126
@Pandry A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
- :x: **CI fails**: `CI image checks / Static checks` (and possibly other checks — see the [Checks tab](https://github.com/apache/airflow/pull/62938/checks) for the full list).
No rush.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
--
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