uranusjr commented on a change in pull request #20425:
URL: https://github.com/apache/airflow/pull/20425#discussion_r772471350
##########
File path: airflow/jobs/scheduler_job.py
##########
@@ -1140,8 +1140,8 @@ def _emit_pool_metrics(self, session: Session = None) ->
None:
pools = models.Pool.slots_stats(session=session)
for pool_name, slot_stats in pools.items():
Stats.gauge(f'pool.open_slots.{pool_name}', slot_stats["open"])
- Stats.gauge(f'pool.queued_slots.{pool_name}',
slot_stats[TaskInstanceState.QUEUED])
- Stats.gauge(f'pool.running_slots.{pool_name}',
slot_stats[TaskInstanceState.RUNNING])
+ Stats.gauge(f'pool.queued_slots.{pool_name}', slot_stats["queued"])
Review comment:
Maybe `TaskInstanceState.QUEUED.value` is better? (Same for below)
##########
File path: airflow/operators/python.py
##########
@@ -173,7 +173,9 @@ def __init__(
self.show_return_value_in_logs = show_return_value_in_logs
def execute(self, context: Context) -> Any:
- context.update(self.op_kwargs)
+ # The casting below is not `perfect`, but we "pretend" that context is
TypedDict via Context.pyi
+ # So no harm done and it will work as expected.
+ context.update(cast(Context, self.op_kwargs))
context['templates_dict'] = self.templates_dict
Review comment:
Another possibility is
```suggestion
context_additions = {"templates_dict": self.templates_dict}
if self.op_kwargs:
context_additions.update(self.op_kwargs)
context = Context(context, **context_additions)
```
It’s probably a good idea to add a function in `airflow.utils.context` for
this. Maybe call it `context_merge`.
##########
File path: airflow/utils/context.pyi
##########
@@ -79,6 +79,7 @@ class Context(TypedDict, total=False):
task_instance: TaskInstance
task_instance_key_str: str
test_mode: bool
+ templates_dict: Optional[dict]
Review comment:
```suggestion
templates_dict: Optional[Mapping[str, Any]]
```
##########
File path: airflow/macros/__init__.py
##########
@@ -66,7 +67,7 @@ def ds_format(ds: str, input_format: str, output_format: str)
-> str:
return datetime.strptime(str(ds), input_format).strftime(output_format)
-def datetime_diff_for_humans(dt: Any, since: Optional[datetime] = None) -> str:
+def datetime_diff_for_humans(dt: Any, since: Optional[Union[datetime,
DateTime]] = None) -> str:
Review comment:
This should not be needed because `Union[datetime, DateTime]` is just
`datetime`. This entire module should only need one change; see #20132.
--
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]
