[AIRFLOW-6088] pass DAG processing runtime as duration to stats #6682
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6682 +/- ##
==========================================
- Coverage 83.82% 83.29% -0.54%
==========================================
Files 668 668
Lines 37543 37542 -1
==========================================
- Hits 31472 31270 -202
- Misses 6071 6272 +201
Continue to review full report at Codecov.
|
XD-DENG
left a comment
XD-DENG
left a comment
There was a problem hiding this comment.
It's a nice catch.
I cross-checked the code & documentation, and LGTM.
Thanks for your contribution!
|
@hoesler Thanks again for the PR! LGTM and I would be more than happy to merge it to master. Before I merge, may we request a favor from you: may you check if there is similar/same issue for other I will also do the same check from my side. After that we are good to go. |
|
Hi @hoesler , airflow/airflow/models/dagrun.py Lines 309 to 310 in 1097892
May you address this together in this PR? After that I can merge immediately. All other related lines in the code base seems ok. Cheers! |
|
Sure, I will add that |
|
found one more |
|
|
||
| td = timezone.utcnow() - ts | ||
| td = td.total_seconds() + ( | ||
| float(td.microseconds) / 1000000) |
There was a problem hiding this comment.
One of the error I found in the CI log is below
1) ERROR: test_cli_list_dags (tests.cli.commands.test_dag_command.TestCliDags)
----------------------------------------------------------------------
Traceback (most recent call last):
tests/cli/commands/test_dag_command.py line 361 in test_cli_list_dags
dag_command.dag_list_dags(args)
airflow/utils/cli.py line 80 in wrapper
return f(*args, **kwargs)
airflow/cli/commands/dag_command.py line 231 in dag_list_dags
print(dagbag.dagbag_report())
airflow/models/dagbag.py line 480 in dagbag_report
duration=sum([o.duration for o in stats]),
TypeError: unsupported operand type(s) for +: 'int' and 'datetime.timedelta'
It should be due to this change in airflow/models/dagbag.py
There was a problem hiding this comment.
For this specific file/change, in order to avoid breaking other logics in other files, I would like to suggest to change in Stats.timing() line instead.
I.e., revert your change in line 422, then change
filename = file_stat.file.split('/')[-1].replace('.py', '')
Stats.timing('dag.loading-duration.{}'.
format(filename),
format(filename),
file_stat.duration)into
filename = file_stat.file.split('/')[-1].replace('.py', '')
Stats.timing('dag.loading-duration.{}'.
format(filename),
format(filename),
timedelta(seconds=file_stat.duration))(timedelta needs to be imported)
Let me know your opinion? Cheers
|
The problem was the sum function, which is basically a reduce(array, +, 0). I had to change the initial value to timedelta() |
|
All tests passed, except |
|
|
|
Cool, thanks @hoesler . To be safe, I restarted the failing test. Will merge once it turns green. |
|
Hi @hoesler , please note I updated the subject of your JIRA ticket to better describe the code change here, also for clearer change log. |
4 participants
Make sure you have checked all steps below.
Jira
Description
runtime var is now a duration object, not a float
Tests
Commits
Documentation