You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ashb (via GitHub)" <gi...@apache.org> on 2025/01/31 11:26:44 UTC

[PR] Revert "Mostly use relative fileloc in dag processor" [airflow]

ashb opened a new pull request, #46308:
URL: https://github.com/apache/airflow/pull/46308

   Reverts apache/airflow#46290
   
   Unfortunately this broke `prepare_file_path_queue` which only showed up in the Kube integration tests, not in this PR (due to selective tests, which 99.9% of the time is fine to not run for this sort of change)
   
   The error we saw was:
   
   ```
   2025-01-31T10:02:53.715990881Z stderr F   File "/home/airflow/.local/lib/python3.10/site-packages/airflow/dag_processing/manager.py", line 317, in _run_parsing_loop
   2025-01-31T10:02:53.715993857Z stderr F     self.prepare_file_path_queue()
   2025-01-31T10:02:53.715996832Z stderr F   File "/home/airflow/.local/lib/python3.10/site-packages/airflow/dag_processing/manager.py", line 899, in prepare_file_path_queue
   2025-01-31T10:02:53.716003114Z stderr F     "\n\t".join(f.path for f in files_paths_to_queue),
   2025-01-31T10:02:53.71600625Z stderr F   File "/home/airflow/.local/lib/python3.10/site-packages/airflow/dag_processing/manager.py", line 899, in <genexpr>
   2025-01-31T10:02:53.716009275Z stderr F     "\n\t".join(f.path for f in files_paths_to_queue),
   2025-01-31T10:02:53.716012241Z stderr F AttributeError: 'DagFileInfo' object has no attribute 'path'
   ```
   
   And while that change was easy enough to fix, (`path` -> `relpath`) I wasn't happy just fixing that path, so I also added typing here:
   
   https://github.com/apache/airflow/blob/9ef89acf85dffe69266b716fa0fa1cfa6246b1f3/airflow/dag_processing/manager.py#L831
   
   ```python
           file_paths: list[DagFileInfo] = []
   ```
   
   And also adding a `-> None:` on the function so mypy type checks it, and then suddenly we get all these errors:
   
   ```
   airflow/dag_processing/manager.py:833: note: Revealed type is "builtins.list[airflow.dag_processing.manager.DagFileInfo]"
   airflow/dag_processing/manager.py:840: error: Invalid index type "DagFileInfo" for "dict[str, datetime]"; expected type "str"  [index]
                           files_with_mtime[file_path] = os.path.getmtime(file_path.absolute_path)
                                            ^~~~~~~~~
   airflow/dag_processing/manager.py:840: error: Incompatible types in assignment (expression has type "float", target has type "datetime")  [assignment]
                           files_with_mtime[file_path] = os.path.getmtime(file_path.absolute_path)
                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   airflow/dag_processing/manager.py:846: error: Argument 1 to "fromtimestamp" of "datetime" has incompatible type "datetime"; expected "float"  [arg-type]
                       file_modified_time = datetime.fromtimestamp(files_with_mtime[file_path], tz=timezone.utc)
                                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   airflow/dag_processing/manager.py:846: error: Invalid index type "DagFileInfo" for "dict[str, datetime]"; expected type "str"  [index]
                       file_modified_time = datetime.fromtimestamp(files_with_mtime[file_path], tz=timezone.utc)
                                                                                    ^~~~~~~~~
   airflow/dag_processing/manager.py:862: note: Revealed type is "builtins.list[airflow.dag_processing.manager.DagFileInfo]"
   airflow/dag_processing/manager.py:865: note: Revealed type is "builtins.dict[builtins.str, datetime.datetime]"
   airflow/dag_processing/manager.py:866: error: Argument 1 to "sorted" has incompatible type "dict[str, datetime]"; expected "Iterable[DagFileInfo]"  [arg-type]
                   file_paths = sorted(files_with_mtime, key=files_with_mtime.get, reverse=True)
                                       ^~~~~~~~~~~~~~~~
   airflow/dag_processing/manager.py:866: error: Argument "key" to "sorted" has incompatible type overloaded function; expected "Callable[[DagFileInfo], SupportsDunderLT[Any] | SupportsDunderGT[Any]]"  [arg-type]
                   file_paths = sorted(files_with_mtime, key=files_with_mtime.get, reverse=True)
                                                             ^~~~~~~~~~~~~~~~~~~~
   airflow/dag_processing/manager.py:867: error: Name "files_paths" is not defined  [name-defined]
                   reveal_type(files_paths)
                               ^
   airflow/dag_processing/manager.py:867: note: Revealed type is "Any"
   airflow/dag_processing/manager.py:895: note: Revealed type is "builtins.list[airflow.dag_processing.manager.DagFileInfo]"
   airflow/dag_processing/manager.py:904: error: "DagFileInfo" has no attribute "relpath"; maybe "rel_path"?  [attr-defined]
                       "\n\t".join(f.relpath for f in files_paths_to_queue),
                                   ^~~~~~~~~
   Found 8 errors in 1 file (checked 1 source file)
   ```
   
   The mtime path is borken, so reverting this for now as it's breaking main.
   
   Sorry @dstandish 


-- 
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] Revert "Mostly use relative fileloc in dag processor" [airflow]

Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #46308:
URL: https://github.com/apache/airflow/pull/46308


-- 
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] Revert "Mostly use relative fileloc in dag processor" [airflow]

Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #46308:
URL: https://github.com/apache/airflow/pull/46308#issuecomment-2626987344

   Lets wait for the tests just to be sure


-- 
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] Revert "Mostly use relative fileloc in dag processor" [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #46308:
URL: https://github.com/apache/airflow/pull/46308#issuecomment-2627475834

   Good catch 🐟 !!


-- 
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] Revert "Mostly use relative fileloc in dag processor" [airflow]

Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #46308:
URL: https://github.com/apache/airflow/pull/46308#issuecomment-2626986818

   Do I wait for tests or just merge it now do you think @amoghrajesh ?


-- 
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