Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W merged PR #52868: URL: https://github.com/apache/airflow/pull/52868 -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
kaxil commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3058905781 #protm -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
jscheffl commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3058859104 > > > @ashb and I had a brief discussion regarding `ApprovalOperator`, which was previously a `HITLOperator` with its options set to `Approve` and `Reject`, and does nothing more. It may surprise some users if they select `Reject` and the process still proceeds. (which is still a valid use case, but users can also achieve them through `HITLBranchOperator`, or inherit `HITLOperator`). So we're thinking of merging `HITLTerminationOperator` into `ApprovelOperator` and treat "Reject" as "Stop" > > > > > > What would be the use-case of the opposite -- i.e. "Reject" but still continue with the following tasks? Similar to Ash, I was assuming the "Approval" to be similar to Circle CI approval step ([source](https://circleci.com/blog/adding-approval-jobs-to-your-ci-pipeline/#:~:text=Just%20like%20that%2C%20selecting%20Approve%20makes%20the%20API%20call%20back%20to%20CircleCI%20to%20approve%20the%20pipeline%20and%20deploy%20your%20code%20to%20production.)) > > One example that comes to mind is the content moderation process described in the AIP-90 document. When a moderator approves content, it would be best for the system to simply terminate without taking any further action. On the other hand, if the content is rejected, it could lead to post deletions or other consequences, depending on the designed use case. Iām okay with it and have already updated the operator, as users can utilize other operators to achieve the same outcome. I assume if you want to have follow-up actions on "Reject" then the branching operator type is more suited. I would see the "Approval" with Reject rather as a function like a ShortCircuit... but no strong feelings. Just feels more intuitive when I re-think... -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3057713352 Hey @guan404ming @sunank200 , i've just updated the dag and rest API code in https://github.com/apache/airflow/pull/52868#issuecomment-3037857107. This should reflect the lates change -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3057690658 Hey @jscheffl , I've addressed all of the things we discussed. Would you mind taking a final round and see if we could merge it to unblock other issues? Thanks a lot! -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3057681769 > > @ashb and I had a brief discussion regarding `ApprovalOperator`, which was previously a `HITLOperator` with its options set to `Approve` and `Reject`, and does nothing more. It may surprise some users if they select `Reject` and the process still proceeds. (which is still a valid use case, but users can also achieve them through `HITLBranchOperator`, or inherit `HITLOperator`). So we're thinking of merging `HITLTerminationOperator` into `ApprovelOperator` and treat "Reject" as "Stop" > > What would be the use-case of the opposite -- i.e. "Reject" but still continue with the following tasks? Similar to Ash, I was assuming the "Approval" to be similar to Circle CI approval step ([source](https://circleci.com/blog/adding-approval-jobs-to-your-ci-pipeline/#:~:text=Just%20like%20that%2C%20selecting%20Approve%20makes%20the%20API%20call%20back%20to%20CircleCI%20to%20approve%20the%20pipeline%20and%20deploy%20your%20code%20to%20production.)) One example I can think of is the content moderation mentioned in the AIP-90 document. It should probably terminate and do nothing when the moderator approves the content. Rejecting the content might incur post deletion or other actions. It really depends on the use case. But I already update the operator as suggested. -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
kaxil commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3056115078 > @ashb and I had a brief discussion regarding `ApprovalOperator`, which was previously a `HITLOperator` with its options set to `Approve` and `Reject`, and does nothing more. It may surprise some users if they select `Reject` and the process still proceeds. (which is still a valid use case, but users can also achieve them through `HITLBranchOperator`, or inherit `HITLOperator`). So we're thinking of merging `HITLTerminationOperator` into `ApprovelOperator` and treat "Reject" as "Stop" What would be the use-case of the opposite -- i.e. "Reject" but still continue with the following tasks? Similar to Ash, I was assuming the "Approval" to be similar to Circle CI approval step ([source](https://circleci.com/blog/adding-approval-jobs-to-your-ci-pipeline/#:~:text=Just%20like%20that%2C%20selecting%20Approve%20makes%20the%20API%20call%20back%20to%20CircleCI%20to%20approve%20the%20pipeline%20and%20deploy%20your%20code%20to%20production.)) -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3055875681 @ashb and I had a brief discussion regarding `ApprovalOperator`, which was previously a `HITLOperator` with its options set to `Approve` and `Reject`, and does nothing more. It may surprise some users if they select `Reject` and the process still proceeds. (which is still a valid use case, but users can also achieve them through `HITLBranchOperator`, or inherit `HITLOperator`). So we're thinking of merging `HITLTerminationOperator` into `ApprovelOperator` and treat "Reject" as "Stop" -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
jscheffl commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2195740421 ## airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py: ## @@ -0,0 +1,77 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from collections.abc import Mapping +from datetime import datetime +from typing import Any + +from pydantic import Field, field_validator + +from airflow.api_fastapi.core_api.base import BaseModel +from airflow.sdk import Param + + +class UpdateHITLResponsePayload(BaseModel): +"""Schema for updating the content of a Human-in-the-loop response.""" + +response_content: list[str] +params_input: Mapping = Field(default_factory=dict) + + +class HITLResponseContentDetail(BaseModel): +"""Response of updating a Human-in-the-loop response.""" + +user_id: str +response_at: datetime +response_content: list[str] +params_input: Mapping = Field(default_factory=dict) + + +class HITLResponseDetail(BaseModel): +"""Schema for Human-in-the-loop response.""" + +ti_id: str + +# Input Request +options: list[str] +subject: str +body: str | None = None +default: list[str] | None = None +multiple: bool = False +params: dict[str, Any] = Field(default_factory=dict) + +# Response Content Detail +user_id: str | None = None +response_at: datetime | None = None +response_content: list[str] | None = None +params_input: dict[str, Any] = Field(default_factory=dict) Review Comment: Ah ,okay I undrstand. You mean mypy foring to add a "not None" check for all functions downstream... mhm yeah. That is a good reason though. I am not too much worried about these few bytes of DB in there. -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
jscheffl commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2195738272 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): Review Comment: HITLDetail is okay for me! -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3053023313 > Some comments - hope you are treating them as constructive - mostly on naming... yeah we know naming is hard. Yep... I know the naming was bad š¤¦āāļø and probably still not great now, but this is the best I can think of now š¤ > Besides docs also an example would be cool, but can also be a follow-up PR! those are tracked in https://github.com/orgs/apache/projects/508/views/1. All the comments listed in https://github.com/apache/airflow/pull/52868#issuecomment-3050955648 has been address and also changed the public endpoint for UI's need (cc @guan404ming @sunank200 , see the updated description). would be great if you could take another look when you're available š -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2194513149 ## airflow-core/src/airflow/api_fastapi/execution_api/routes/__init__.py: ## @@ -48,5 +49,6 @@ ) authenticated_router.include_router(variables.router, prefix="/variables", tags=["Variables"]) authenticated_router.include_router(xcoms.router, prefix="/xcoms", tags=["XComs"]) +authenticated_router.include_router(hitl.router, prefix="/hitl-responses", tags=["Human in the Loop"]) Review Comment: just updated it to `hitl-details` -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2194416698
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.exceptions import AirflowOptionalProviderFeatureException
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise AirflowOptionalProviderFeatureException("Human in the loop
functionality needs Airflow 3.1+.")
+
+
+from collections.abc import Collection, Mapping
+from datetime import datetime, timezone
+from typing import TYPE_CHECKING, Any
+
+from airflow.models import SkipMixin
+from airflow.models.baseoperator import BaseOperator
+from airflow.providers.standard.exceptions import HITLTriggerEventError
+from airflow.providers.standard.triggers.hitl import HITLTrigger,
HITLTriggerEventSuccessPayload
+from airflow.sdk.definitions.param import ParamsDict
+from airflow.sdk.execution_time.hitl import add_hitl_response
+
+if TYPE_CHECKING:
+from airflow.sdk.definitions.context import Context
+
+
+class HITLOperator(BaseOperator):
+"""
+Base class for all Human-in-the-loop Operators to inherit from.
+
+:param subject: Headline/subject presented to the user for the interaction
task.
+:param options: List of options that the an user can select from to
complete the task.
+:param body: descriptive text that might give background, hints or can
provide background or summary of
+details that are needed to decide.
Review Comment:
updated
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2194406793
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.exceptions import AirflowOptionalProviderFeatureException
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise AirflowOptionalProviderFeatureException("Human in the loop
functionality needs Airflow 3.1+.")
+
+
+from collections.abc import Collection, Mapping
+from datetime import datetime, timezone
+from typing import TYPE_CHECKING, Any
+
+from airflow.models import SkipMixin
+from airflow.models.baseoperator import BaseOperator
+from airflow.providers.standard.exceptions import HITLTriggerEventError
+from airflow.providers.standard.triggers.hitl import HITLTrigger,
HITLTriggerEventSuccessPayload
+from airflow.sdk.definitions.param import ParamsDict
+from airflow.sdk.execution_time.hitl import add_hitl_response
+
+if TYPE_CHECKING:
+from airflow.sdk.definitions.context import Context
+
+
+class HITLOperator(BaseOperator):
+"""
+Base class for all Human-in-the-loop Operators to inherit from.
+
+:param subject: Headline/subject presented to the user for the interaction
task.
+:param options: List of options that the an user can select from to
complete the task.
+:param body: descriptive text that might give background, hints or can
provide background or summary of
+details that are needed to decide.
+:param default: The default option and the option that is taken if timeout
is passed.
+:param multiple: Whether the user can select one or multiple options.
+:param params: dictionary of parameter definitions that are in the format
of Dag params such that
+a Form Field can be rendered. Entered data is validated (schema,
required fields) like for a Dag run
+and added to XCom of the task result.
+"""
+
+template_fields: Collection[str] = ("subject", "body")
+
+def __init__(
+self,
+*,
+subject: str,
+options: list[str],
+body: str | None = None,
+default: str | list[str] | None = None,
+multiple: bool = False,
+params: ParamsDict | dict[str, Any] | None = None,
+**kwargs,
+) -> None:
+super().__init__(**kwargs)
+self.subject = subject
+self.body = body
+
+self.options = options
+# allow defaults to store more than one options when multiple=True
+self.default = [default] if isinstance(default, str) else default
+self.multiple = multiple
+
+self.params: ParamsDict | dict = params or {}
Review Comment:
updated
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2194399229 ## airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py: ## @@ -0,0 +1,71 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Add Human In the Loop Response table. + +Revision ID: 40f7c30a228b +Revises: ffdb0566c7c0 +Create Date: 2025-07-04 15:05:19.459197 + +""" + +from __future__ import annotations + +import sqlalchemy_jsonfield +from alembic import op +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql + +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + +# revision identifiers, used by Alembic. +revision = "40f7c30a228b" +down_revision = "ffdb0566c7c0" +branch_labels = None +depends_on = None +airflow_version = "3.1.0" + + +def upgrade(): +"""Add Human In the Loop Response table.""" +op.create_table( +"hitl_response", +Column( +"ti_id", Review Comment: updated! -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3050971454 oh, there's another thing I need to check for the UI side. "How we retrieve ti id from the front end" -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193902943 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): +"""Human-in-the-loop received response.""" + +__tablename__ = "hitl_response" +ti_id = Column( +String(36).with_variant(postgresql.UUID(as_uuid=False), "postgresql"), +primary_key=True, +nullable=False, +) + +# Input Request Review Comment: indeed. Thanks! just updated -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on PR #52868: URL: https://github.com/apache/airflow/pull/52868#issuecomment-3050955648 To keep myself on track. here're the comments to discuss and addresse ## What I'll address today 2. https://github.com/apache/airflow/pull/52868#discussion_r2193883283 3. https://github.com/apache/airflow/pull/52868#discussion_r2193413166 4. https://github.com/apache/airflow/pull/52868#discussion_r2193904263 5. https://github.com/apache/airflow/pull/52868#discussion_r2193429679 ## What might need some more discussion https://github.com/apache/airflow/pull/52868#discussion_r2193399844 https://github.com/apache/airflow/pull/52868#discussion_r2193405905 https://github.com/apache/airflow/pull/52868#discussion_r2193883283 -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193905930
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.exceptions import AirflowOptionalProviderFeatureException
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise AirflowOptionalProviderFeatureException("Human in the loop
functionality needs Airflow 3.1+.")
+
+
+from collections.abc import Collection, Mapping
+from datetime import datetime, timezone
+from typing import TYPE_CHECKING, Any
+
+from airflow.models import SkipMixin
+from airflow.models.baseoperator import BaseOperator
+from airflow.providers.standard.exceptions import HITLTriggerEventError
+from airflow.providers.standard.triggers.hitl import HITLTrigger,
HITLTriggerEventSuccessPayload
+from airflow.sdk.definitions.param import ParamsDict
+from airflow.sdk.execution_time.hitl import add_hitl_response
+
+if TYPE_CHECKING:
+from airflow.sdk.definitions.context import Context
+
+
+class HITLOperator(BaseOperator):
+"""
+Base class for all Human-in-the-loop Operators to inherit from.
+
+:param subject: Headline/subject presented to the user for the interaction
task.
+:param options: List of options that the an user can select from to
complete the task.
+:param body: descriptive text that might give background, hints or can
provide background or summary of
+details that are needed to decide.
+:param default: The default option and the option that is taken if timeout
is passed.
+:param multiple: Whether the user can select one or multiple options.
+:param params: dictionary of parameter definitions that are in the format
of Dag params such that
+a Form Field can be rendered. Entered data is validated (schema,
required fields) like for a Dag run
+and added to XCom of the task result.
+"""
+
+template_fields: Collection[str] = ("subject", "body")
+
+def __init__(
+self,
+*,
+subject: str,
+options: list[str],
+body: str | None = None,
+default: str | list[str] | None = None,
+multiple: bool = False,
+params: ParamsDict | dict[str, Any] | None = None,
+**kwargs,
+) -> None:
+super().__init__(**kwargs)
+self.subject = subject
+self.body = body
+
+self.options = options
+# allow defaults to store more than one options when multiple=True
+self.default = [default] if isinstance(default, str) else default
+self.multiple = multiple
+
+self.params: ParamsDict | dict = params or {}
Review Comment:
ah... right... will address it!
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193904263
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,206 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.exceptions import AirflowOptionalProviderFeatureException
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise AirflowOptionalProviderFeatureException("Human in the loop
functionality needs Airflow 3.1+.")
+
+
+from collections.abc import Collection, Mapping
+from datetime import datetime, timezone
+from typing import TYPE_CHECKING, Any
+
+from airflow.models import SkipMixin
+from airflow.models.baseoperator import BaseOperator
+from airflow.providers.standard.exceptions import HITLTriggerEventError
+from airflow.providers.standard.triggers.hitl import HITLTrigger,
HITLTriggerEventSuccessPayload
+from airflow.sdk.definitions.param import ParamsDict
+from airflow.sdk.execution_time.hitl import add_hitl_response
+
+if TYPE_CHECKING:
+from airflow.sdk.definitions.context import Context
+
+
+class HITLOperator(BaseOperator):
+"""
+Base class for all Human-in-the-loop Operators to inherit from.
+
+:param subject: Headline/subject presented to the user for the interaction
task.
+:param options: List of options that the an user can select from to
complete the task.
+:param body: descriptive text that might give background, hints or can
provide background or summary of
+details that are needed to decide.
Review Comment:
I'll update the description. Thanks! From the backend perspective, I think
we'll just store plain text and let UI decide whether to render?
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193901317 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): Review Comment: but let's keep the naming thing discussed in https://github.com/apache/airflow/pull/52868#discussion_r2193883283 -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193900880 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): Review Comment: I'll change it to `HITLDetail` for now because of the time zone difference, but I'm open to other name suggestions since this one isn't ideal either. I think `HITLTask` could be confusing, as it doesn't represent a task, and we already have the concept of a task in Airflow. On its own, `HITL` doesnāt make much sense to me, as it doesnāt refer to a š¤¦āāļø -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193897322 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): +"""Human-in-the-loop received response.""" + +__tablename__ = "hitl_response" +ti_id = Column( +String(36).with_variant(postgresql.UUID(as_uuid=False), "postgresql"), Review Comment: yep will be done along with https://github.com/apache/airflow/pull/52868#discussion_r2193883951 -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193896400
##
airflow-core/src/airflow/models/hitl.py:
##
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import sqlalchemy_jsonfield
+from sqlalchemy import Boolean, Column, String, Text
+from sqlalchemy.dialects import postgresql
+from sqlalchemy.ext.hybrid import hybrid_property
+
+from airflow.models.base import Base
+from airflow.settings import json
+from airflow.utils.sqlalchemy import UtcDateTime
+
+
+class HITLResponseModel(Base):
+"""Human-in-the-loop received response."""
+
+__tablename__ = "hitl_response"
+ti_id = Column(
+String(36).with_variant(postgresql.UUID(as_uuid=False), "postgresql"),
+primary_key=True,
+nullable=False,
+)
+
+# Input Request
+options = Column(sqlalchemy_jsonfield.JSONField(json=json), nullable=False)
+subject = Column(Text, nullable=False)
+body = Column(Text, nullable=True)
+default = Column(sqlalchemy_jsonfield.JSONField(json=json), nullable=True)
+multiple = Column(Boolean, unique=False, default=False)
+params = Column(sqlalchemy_jsonfield.JSONField(json=json), nullable=False,
default={})
+
+# Response Content Detail
+response_at = Column(UtcDateTime, nullable=True)
+user_id = Column(String(128), nullable=True)
+response_content = Column(
+sqlalchemy_jsonfield.JSONField(json=json),
+nullable=True,
+default=None,
+)
+params_input = Column(sqlalchemy_jsonfield.JSONField(json=json),
nullable=False, default={})
Review Comment:
let's keep this discussion in
https://github.com/apache/airflow/pull/52868#discussion_r2193880430
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193896122
##
airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py:
##
@@ -0,0 +1,71 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Add Human In the Loop Response table.
+
+Revision ID: 40f7c30a228b
+Revises: ffdb0566c7c0
+Create Date: 2025-07-04 15:05:19.459197
+
+"""
+
+from __future__ import annotations
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Boolean, Column, String, Text
+from sqlalchemy.dialects import postgresql
+
+from airflow.settings import json
+from airflow.utils.sqlalchemy import UtcDateTime
+
+# revision identifiers, used by Alembic.
+revision = "40f7c30a228b"
+down_revision = "ffdb0566c7c0"
+branch_labels = None
+depends_on = None
+airflow_version = "3.1.0"
+
+
+def upgrade():
+"""Add Human In the Loop Response table."""
+op.create_table(
+"hitl_response",
Review Comment:
let's keep this discussion in
https://github.com/apache/airflow/pull/52868#discussion_r2193883283
##
airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py:
##
@@ -0,0 +1,71 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Add Human In the Loop Response table.
+
+Revision ID: 40f7c30a228b
+Revises: ffdb0566c7c0
+Create Date: 2025-07-04 15:05:19.459197
+
+"""
+
+from __future__ import annotations
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Boolean, Column, String, Text
+from sqlalchemy.dialects import postgresql
+
+from airflow.settings import json
+from airflow.utils.sqlalchemy import UtcDateTime
+
+# revision identifiers, used by Alembic.
+revision = "40f7c30a228b"
+down_revision = "ffdb0566c7c0"
+branch_labels = None
+depends_on = None
+airflow_version = "3.1.0"
+
+
+def upgrade():
+"""Add Human In the Loop Response table."""
+op.create_table(
+"hitl_response",
+Column(
+"ti_id",
+String(length=36).with_variant(postgresql.UUID(), "postgresql"),
+primary_key=True,
+nullable=False,
+),
+Column("options", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False),
+Column("subject", Text, nullable=False),
+Column("body", Text, nullable=True),
+Column("default", sqlalchemy_jsonfield.JSONField(json=json),
nullable=True),
+Column("multiple", Boolean, unique=False, default=False),
+Column("params", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False, default={}),
+Column("response_at", UtcDateTime, nullable=True),
+Column("user_id", String(128), nullable=True),
+Column("response_content", sqlalchemy_jsonfield.JSONField(json=json),
nullable=True),
+Column("params_input", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False, default={}),
Review Comment:
let's keep this discussion in
https://github.com/apache/airflow/pull/52868#discussion_r2193880430
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193883283 ## airflow-core/src/airflow/api_fastapi/execution_api/routes/__init__.py: ## @@ -48,5 +49,6 @@ ) authenticated_router.include_router(variables.router, prefix="/variables", tags=["Variables"]) authenticated_router.include_router(xcoms.router, prefix="/xcoms", tags=["XComs"]) +authenticated_router.include_router(hitl.router, prefix="/hitl-responses", tags=["Human in the Loop"]) Review Comment: what bugs me is that `hitl` itself is not an object or resource. `hitl-response` is probably not really better. I'll go with `hitl-detail` for now -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193895294 ## airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py: ## @@ -0,0 +1,71 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Add Human In the Loop Response table. + +Revision ID: 40f7c30a228b +Revises: ffdb0566c7c0 +Create Date: 2025-07-04 15:05:19.459197 + +""" + +from __future__ import annotations + +import sqlalchemy_jsonfield +from alembic import op +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql + +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + +# revision identifiers, used by Alembic. +revision = "40f7c30a228b" +down_revision = "ffdb0566c7c0" +branch_labels = None +depends_on = None +airflow_version = "3.1.0" + + +def upgrade(): +"""Add Human In the Loop Response table.""" +op.create_table( +"hitl_response", Review Comment: not sure whether `hitl_detail` is better. making it a `task` sounds misleading š¤ `detail` is vague and not the best but might be better than what we have now IMO -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193886724 ## providers/standard/tests/unit/standard/api_fastapi/__init__.py: ## Review Comment: right... let me remove it -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193884863
##
airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py:
##
@@ -0,0 +1,71 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Add Human In the Loop Response table.
+
+Revision ID: 40f7c30a228b
+Revises: ffdb0566c7c0
+Create Date: 2025-07-04 15:05:19.459197
+
+"""
+
+from __future__ import annotations
+
+import sqlalchemy_jsonfield
+from alembic import op
+from sqlalchemy import Boolean, Column, String, Text
+from sqlalchemy.dialects import postgresql
+
+from airflow.settings import json
+from airflow.utils.sqlalchemy import UtcDateTime
+
+# revision identifiers, used by Alembic.
+revision = "40f7c30a228b"
+down_revision = "ffdb0566c7c0"
+branch_labels = None
+depends_on = None
+airflow_version = "3.1.0"
+
+
+def upgrade():
+"""Add Human In the Loop Response table."""
+op.create_table(
+"hitl_response",
+Column(
+"ti_id",
+String(length=36).with_variant(postgresql.UUID(), "postgresql"),
+primary_key=True,
+nullable=False,
+),
+Column("options", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False),
+Column("subject", Text, nullable=False),
+Column("body", Text, nullable=True),
+Column("default", sqlalchemy_jsonfield.JSONField(json=json),
nullable=True),
+Column("multiple", Boolean, unique=False, default=False),
+Column("params", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False, default={}),
+Column("response_at", UtcDateTime, nullable=True),
+Column("user_id", String(128), nullable=True),
+Column("response_content", sqlalchemy_jsonfield.JSONField(json=json),
nullable=True),
+Column("params_input", sqlalchemy_jsonfield.JSONField(json=json),
nullable=False, default={}),
Review Comment:
let's keep this discussion in one place
https://github.com/apache/airflow/pull/52868#discussion_r2193880430
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193883951 ## airflow-core/src/airflow/migrations/versions/0076_3_1_0_add_human_in_the_loop_response.py: ## @@ -0,0 +1,71 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Add Human In the Loop Response table. + +Revision ID: 40f7c30a228b +Revises: ffdb0566c7c0 +Create Date: 2025-07-04 15:05:19.459197 + +""" + +from __future__ import annotations + +import sqlalchemy_jsonfield +from alembic import op +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql + +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + +# revision identifiers, used by Alembic. +revision = "40f7c30a228b" +down_revision = "ffdb0566c7c0" +branch_labels = None +depends_on = None +airflow_version = "3.1.0" + + +def upgrade(): +"""Add Human In the Loop Response table.""" +op.create_table( +"hitl_response", +Column( +"ti_id", Review Comment: right... need to add it back. Will do. Thanks! -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193883283 ## airflow-core/src/airflow/api_fastapi/execution_api/routes/__init__.py: ## @@ -48,5 +49,6 @@ ) authenticated_router.include_router(variables.router, prefix="/variables", tags=["Variables"]) authenticated_router.include_router(xcoms.router, prefix="/xcoms", tags=["XComs"]) +authenticated_router.include_router(hitl.router, prefix="/hitl-responses", tags=["Human in the Loop"]) Review Comment: what bugs me is that `hitl` itself is not an object or resource. `hitl-response` is probably not really better. I'll go with `hitl-task` for now -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193881157
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##
@@ -0,0 +1,144 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from uuid import UUID
+
+import structlog
+from fastapi import Depends, HTTPException, status
+from sqlalchemy import select
+
+from airflow.api_fastapi.auth.managers.models.resource_details import
DagAccessEntity
+from airflow.api_fastapi.common.db.common import SessionDep, paginated_select
+from airflow.api_fastapi.common.router import AirflowRouter
+from airflow.api_fastapi.core_api.datamodels.hitl import (
+HITLResponseContentDetail,
+HITLResponseDetail,
+HITLResponseDetailCollection,
+UpdateHITLResponsePayload,
+)
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
+from airflow.api_fastapi.core_api.security import GetUserDep,
ReadableTIFilterDep, requires_access_dag
+from airflow.models.hitl import HITLResponseModel
+from airflow.models.taskinstance import TaskInstance as TI
+from airflow.utils import timezone
+
+hitl_router = AirflowRouter(tags=["HumanInTheLoop"], prefix="/hitl-responses")
+
+log = structlog.get_logger(__name__)
+
+
+@hitl_router.patch(
+"/{task_instance_id}",
+responses=create_openapi_http_exception_doc(
+[
+status.HTTP_404_NOT_FOUND,
+status.HTTP_409_CONFLICT,
+]
+),
+dependencies=[
+Depends(requires_access_dag(method="GET",
access_entity=DagAccessEntity.TASK_INSTANCE)),
+],
+)
+def update_hitl_response(
+task_instance_id: UUID,
+update_hitl_response_payload: UpdateHITLResponsePayload,
+user: GetUserDep,
+session: SessionDep,
+) -> HITLResponseContentDetail:
+"""Update a Human-in-the-loop response."""
+ti_id_str = str(task_instance_id)
+hitl_response_model = session.scalar(
+select(HITLResponseModel).where(HITLResponseModel.ti_id == ti_id_str)
+)
+if not hitl_response_model:
+raise HTTPException(
+status.HTTP_404_NOT_FOUND,
+f"Human-in-the-loop Response does not exist for Task Instance with
id {ti_id_str}",
+)
+
+if hitl_response_model.response_received:
Review Comment:
Probably let's handle it in another PR. I feel this one might need more
discussion
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2193880430 ## airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py: ## @@ -0,0 +1,77 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from collections.abc import Mapping +from datetime import datetime +from typing import Any + +from pydantic import Field, field_validator + +from airflow.api_fastapi.core_api.base import BaseModel +from airflow.sdk import Param + + +class UpdateHITLResponsePayload(BaseModel): +"""Schema for updating the content of a Human-in-the-loop response.""" + +response_content: list[str] +params_input: Mapping = Field(default_factory=dict) + + +class HITLResponseContentDetail(BaseModel): +"""Response of updating a Human-in-the-loop response.""" + +user_id: str +response_at: datetime +response_content: list[str] +params_input: Mapping = Field(default_factory=dict) + + +class HITLResponseDetail(BaseModel): +"""Schema for Human-in-the-loop response.""" + +ti_id: str + +# Input Request +options: list[str] +subject: str +body: str | None = None +default: list[str] | None = None +multiple: bool = False +params: dict[str, Any] = Field(default_factory=dict) + +# Response Content Detail +user_id: str | None = None +response_at: datetime | None = None +response_content: list[str] | None = None +params_input: dict[str, Any] = Field(default_factory=dict) Review Comment: I"m not sure š¤ Making it a dict makes `params` and `params_input` handling and validation way easier. Unless we're worrying unnecessary db storage, I think the current setup might be better š¤ -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
jscheffl commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2193399844
##
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py:
##
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from collections.abc import Mapping
+from datetime import datetime
+from typing import Any
+
+from pydantic import Field, field_validator
+
+from airflow.api_fastapi.core_api.base import BaseModel
+from airflow.sdk import Param
+
+
+class UpdateHITLResponsePayload(BaseModel):
+"""Schema for updating the content of a Human-in-the-loop response."""
+
+response_content: list[str]
+params_input: Mapping = Field(default_factory=dict)
+
+
+class HITLResponseContentDetail(BaseModel):
+"""Response of updating a Human-in-the-loop response."""
+
+user_id: str
+response_at: datetime
+response_content: list[str]
+params_input: Mapping = Field(default_factory=dict)
+
+
+class HITLResponseDetail(BaseModel):
+"""Schema for Human-in-the-loop response."""
+
+ti_id: str
+
+# Input Request
+options: list[str]
+subject: str
+body: str | None = None
+default: list[str] | None = None
+multiple: bool = False
+params: dict[str, Any] = Field(default_factory=dict)
+
+# Response Content Detail
+user_id: str | None = None
+response_at: datetime | None = None
+response_content: list[str] | None = None
+params_input: dict[str, Any] = Field(default_factory=dict)
Review Comment:
That field should also be null-able as long as no user input made (like the
fields above)?
##
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##
@@ -0,0 +1,144 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from uuid import UUID
+
+import structlog
+from fastapi import Depends, HTTPException, status
+from sqlalchemy import select
+
+from airflow.api_fastapi.auth.managers.models.resource_details import
DagAccessEntity
+from airflow.api_fastapi.common.db.common import SessionDep, paginated_select
+from airflow.api_fastapi.common.router import AirflowRouter
+from airflow.api_fastapi.core_api.datamodels.hitl import (
+HITLResponseContentDetail,
+HITLResponseDetail,
+HITLResponseDetailCollection,
+UpdateHITLResponsePayload,
+)
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
+from airflow.api_fastapi.core_api.security import GetUserDep,
ReadableTIFilterDep, requires_access_dag
+from airflow.models.hitl import HITLResponseModel
+from airflow.models.taskinstance import TaskInstance as TI
+from airflow.utils import timezone
+
+hitl_router = AirflowRouter(tags=["HumanInTheLoop"], prefix="/hitl-responses")
+
+log = structlog.get_logger(__name__)
+
+
+@hitl_router.patch(
+"/{task_instance_id}",
+responses=create_openapi_http_exception_doc(
+[
+status.HTTP_404_NOT_FOUND,
+status.HTTP_409_CONFLICT,
+]
+),
+dependencies=[
+Depends(requires_access_dag(method="GET",
access_entity=DagAccessEntity.TASK_INSTANCE)),
+],
+)
+def update_hitl_response(
+task_instance_id: UUID,
+update_hitl_response_payload: UpdateHITLResponsePayload,
+user: GetUserDep,
+session: SessionDep,
+) -> HITLResponseContentDetail:
+"""Update a Human-in-the-loop response."""
+ti_id_str = str(task_instan
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2192481870
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,207 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise ImportError("Human in the loop functionality needs Airflow 3.1+.")
Review Comment:
updated
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2191865815 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): Review Comment: Originally, `HITLResponseModel` was to reducing confusion elsewhere. let me check whether I can rename them all without confusion -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868: URL: https://github.com/apache/airflow/pull/52868#discussion_r2191862786 ## airflow-core/src/airflow/models/hitl.py: ## @@ -0,0 +1,59 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import sqlalchemy_jsonfield +from sqlalchemy import Boolean, Column, String, Text +from sqlalchemy.dialects import postgresql +from sqlalchemy.ext.hybrid import hybrid_property + +from airflow.models.base import Base +from airflow.settings import json +from airflow.utils.sqlalchemy import UtcDateTime + + +class HITLResponseModel(Base): Review Comment: HITL can be confusing on its own. What does HumanInTheLoop table mean? While `HITLResponse` isn't a perfect term, I am referring to the response needed in a Human In The Loop process. -- 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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
Lee-W commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2191856045
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,207 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise ImportError("Human in the loop functionality needs Airflow 3.1+.")
Review Comment:
indeed. let me update it š
--
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]
Re: [PR] Add Human-in-the-loop logic to core Airflow and implement `HITLOperator`, `ApprovalOperator`, `HITLEntryOperator` in standard provider [airflow]
ephraimbuddy commented on code in PR #52868:
URL: https://github.com/apache/airflow/pull/52868#discussion_r2191810374
##
airflow-core/src/airflow/models/hitl.py:
##
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import sqlalchemy_jsonfield
+from sqlalchemy import Boolean, Column, String, Text
+from sqlalchemy.dialects import postgresql
+from sqlalchemy.ext.hybrid import hybrid_property
+
+from airflow.models.base import Base
+from airflow.settings import json
+from airflow.utils.sqlalchemy import UtcDateTime
+
+
+class HITLResponseModel(Base):
Review Comment:
I feel `HITLResponse(Base)` without the model is enough since we already
know this to be an sqlalchemy model. I also wonder if we should just have it as
`HITL(Base)` why the response?
##
providers/standard/src/airflow/providers/standard/operators/hitl.py:
##
@@ -0,0 +1,207 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from airflow.providers.standard.version_compat import AIRFLOW_V_3_1_PLUS
+
+if not AIRFLOW_V_3_1_PLUS:
+raise ImportError("Human in the loop functionality needs Airflow 3.1+.")
Review Comment:
Maybe, `AirflowOptionalProviderFeatureException` is more suitable here?
--
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]
