You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jscheffl (via GitHub)" <gi...@apache.org> on 2024/11/02 20:55:51 UTC
[PR] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
jscheffl opened a new pull request, #43611:
URL: https://github.com/apache/airflow/pull/43611
This is an alternative PR to #42410 via not converting the priority_weight to float but ensure no roll-over in int is happening - by capping INT values to database limits.
There are several attempts to fix this problem:
- https://github.com/apache/airflow/pull/22784
- https://github.com/apache/airflow/pull/34168
- https://github.com/apache/airflow/pull/37990
- https://github.com/apache/airflow/pull/38125
- https://github.com/apache/airflow/pull/38160
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #43611:
URL: https://github.com/apache/airflow/pull/43611
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #43611:
URL: https://github.com/apache/airflow/pull/43611#discussion_r1840480219
##########
task_sdk/src/airflow/sdk/definitions/baseoperator.py:
##########
@@ -869,6 +874,11 @@ def __init__(
validate_instance_args(self, BASEOPERATOR_ARGS_EXPECTED_TYPES)
+ # Ensure priority_weight is within the valid range
+ self.priority_weight = max(
+ MINIMUM_PRIORITY_WEIGHT, min(MAXIMUM_PRIORITY_WEIGHT, self.priority_weight)
+ )
Review Comment:
Shoud we use `db_safe_priority` here instead of re-impl?
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #43611:
URL: https://github.com/apache/airflow/pull/43611#discussion_r1841057314
##########
task_sdk/src/airflow/sdk/definitions/baseoperator.py:
##########
@@ -869,6 +874,11 @@ def __init__(
validate_instance_args(self, BASEOPERATOR_ARGS_EXPECTED_TYPES)
+ # Ensure priority_weight is within the valid range
+ self.priority_weight = max(
+ MINIMUM_PRIORITY_WEIGHT, min(MAXIMUM_PRIORITY_WEIGHT, self.priority_weight)
+ )
Review Comment:
Would like to but the implementaiton currently is in `airflow-utils` - I asusme we want to have task SDK split apart from core?
Still what I can do is implement the same in task-SDK as utility... the old abstract/base operator in core will go away once AIP-72 is complete, correct?
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #43611:
URL: https://github.com/apache/airflow/pull/43611#issuecomment-2453125823
Besides some adjustments because of rework of classes in regards of TeskSDK I think this can also be back-ported to Ariflow 2.10 line to prevent errors in roll-over. Therefore tagging as 2.10.4 milestone... but backport would most likely a re-write of this PR.
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #43611:
URL: https://github.com/apache/airflow/pull/43611#issuecomment-2470378085
should we have a newsfragment (just in case) just explaining what happened?
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #43611:
URL: https://github.com/apache/airflow/pull/43611#discussion_r1842884469
##########
task_sdk/src/airflow/sdk/definitions/baseoperator.py:
##########
@@ -869,6 +874,11 @@ def __init__(
validate_instance_args(self, BASEOPERATOR_ARGS_EXPECTED_TYPES)
+ # Ensure priority_weight is within the valid range
+ self.priority_weight = max(
+ MINIMUM_PRIORITY_WEIGHT, min(MAXIMUM_PRIORITY_WEIGHT, self.priority_weight)
+ )
Review Comment:
Okay, if it is okay to keep it coupled for the moment... consolidated the calls.
--
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] Ensure priority weight is capped at 32-bit integer to prevent roll-over [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #43611:
URL: https://github.com/apache/airflow/pull/43611#discussion_r1842878453
##########
task_sdk/src/airflow/sdk/definitions/baseoperator.py:
##########
@@ -869,6 +874,11 @@ def __init__(
validate_instance_args(self, BASEOPERATOR_ARGS_EXPECTED_TYPES)
+ # Ensure priority_weight is within the valid range
+ self.priority_weight = max(
+ MINIMUM_PRIORITY_WEIGHT, min(MAXIMUM_PRIORITY_WEIGHT, self.priority_weight)
+ )
Review Comment:
There's still loads of cross imports to fix up, so for now I'd say have one version
--
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