You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "IKholopov (via GitHub)" <gi...@apache.org> on 2025/02/18 02:14:51 UTC
[PR] Migrate DagCode to use relative path for fileloc [airflow]
IKholopov opened a new pull request, #46844:
URL: https://github.com/apache/airflow/pull/46844
Related: #45623
Making DagCode model use relative fileloc of a Dag instead of the absolute one*.
*unit test mode fallbacks to absolute paths still
The main motivation for allowing fallbacks is that there are still a lot of options to define a Dag outside of a bundle and sync it to DB (DagBag without bundle_path, Dag defined directly in a test code or imported module). If bundle path would've been required to be passed for those definition directly - that would be a hard to automate breaking change for (end users) tests code. Compared to `bundle_name` or `code_reader` (introduced here) there is no universal semi-valid path that could've been substituted to cover almost all cases with some fixture.
One alternative that I've considered - instantiate fallback `relative_path -> absolute_path` even earlier and consider `bundle_path == Path("")`, but found it even more error-obscuring compared to explicit allow-listing fallbacks in a fixed number of places.
If my assessment of the issue with using `DagBag/SDM.write_dag` in Dag test code is incorrect, and, for ex. we would be better if we forced everyone to set bundle_path in DagBag & pass dags with fully initialized relative paths, please let me know.
Other changes:
- Inverting control of resolving the full path and reading a code from file to `dag_processing` and DagBag (to avoid tying DagCode model with bundle paths and simplify supplying the src code in tests)
<!-- 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] Migrate DagCode to use relative path for fileloc [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #46844:
URL: https://github.com/apache/airflow/pull/46844#discussion_r1959516663
##########
airflow/models/dagcode.py:
##########
@@ -66,26 +69,41 @@ class DagCode(Base):
)
dag_version = relationship("DagVersion", back_populates="dag_code", uselist=False)
- def __init__(self, dag_version, full_filepath: str, source_code: str | None = None):
+ def __init__(self, dag_version, fileloc: str, source_code: str | None = None):
self.dag_version = dag_version
- self.fileloc = full_filepath
+ self.fileloc = fileloc
self.source_code = source_code or DagCode.code(self.dag_version.dag_id)
self.source_code_hash = self.dag_source_hash(self.source_code)
self.dag_id = dag_version.dag_id
@classmethod
@provide_session
- def write_code(cls, dag_version: DagVersion, fileloc: str, session: Session = NEW_SESSION) -> DagCode:
+ def write_code(
+ cls,
+ dag_version: DagVersion,
+ rel_fileloc: str,
Review Comment:
dag_version already have connection to dag table. We should get the relative fileloc from the dag table: `dag_version.dag.relative_fileloc`
--
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] Migrate DagCode to use relative path for fileloc [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #46844:
URL: https://github.com/apache/airflow/pull/46844#issuecomment-2833718548
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your 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] Migrate DagCode to use relative path for fileloc [airflow]
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on code in PR #46844:
URL: https://github.com/apache/airflow/pull/46844#discussion_r1959516663
##########
airflow/models/dagcode.py:
##########
@@ -66,26 +69,41 @@ class DagCode(Base):
)
dag_version = relationship("DagVersion", back_populates="dag_code", uselist=False)
- def __init__(self, dag_version, full_filepath: str, source_code: str | None = None):
+ def __init__(self, dag_version, fileloc: str, source_code: str | None = None):
self.dag_version = dag_version
- self.fileloc = full_filepath
+ self.fileloc = fileloc
self.source_code = source_code or DagCode.code(self.dag_version.dag_id)
self.source_code_hash = self.dag_source_hash(self.source_code)
self.dag_id = dag_version.dag_id
@classmethod
@provide_session
- def write_code(cls, dag_version: DagVersion, fileloc: str, session: Session = NEW_SESSION) -> DagCode:
+ def write_code(
+ cls,
+ dag_version: DagVersion,
+ rel_fileloc: str,
Review Comment:
dag_version already have connection to dag table. We should get the relative fileloc from the dag table: `dag_version.dag_model.relative_fileloc`
--
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] Migrate DagCode to use relative path for fileloc [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #46844: Migrate DagCode to use relative path for fileloc
URL: https://github.com/apache/airflow/pull/46844
--
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