Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor new Flow trigger conditions to use a new Flow trigger property preconditions with flows, where and timeWindow subproperties #5856

Open
anna-geller opened this issue Nov 8, 2024 · 2 comments · May be fixed by #5903
Assignees
Labels
area/backend Needs backend code changes enhancement New feature or request

Comments

@anna-geller
Copy link
Member

anna-geller commented Nov 8, 2024

Feature description

Context:

  1. We got feedback that some people considered the timeSLA property confusing → timeWindow
  2. We identified a path forward to add a dedicated flow-level sla property: Add flow-level SLA property #5857
  3. The addition of Condition to all conditions is not strictly necessary (validated with Ludo)

We'll refactor as show in this example:

id: myflow42
namespace: company.team
description: |
  Next steps:
    1. Remove the LABEL magic syntax with label:value for now
    2. Add labels to the preconditions.flows array map
    3. Refactor to include preconditions with flows and where
    
    EQUAL_TO,
    NOT_EQUAL_TO,
    IN,
    NOT_IN,
    IS_TRUE,
    IS_FALSE,
    IS_NULL,
    IS_NOT_NULL,
    STARTS_WITH,
    ENDS_WITH,
    REGEX,
    CONTAINS,
    EXPRESSION is a field, not a zype, the type is then IS_TRUE

triggers:
  - id: wait_for_upstream
    type: io.kestra.plugin.core.trigger.Flow
    conditions:
      - type: io.kestra.plugin.core.condition.HasRetryAttemptCondition
    preconditions:
      timeWindow:
        type: DURATION_WINDOW
        window: PT1D
      flows: # AND by default for all list elements
        - namespace: company.team
          flowId: myflow1
          states: [SUCCESS, WARNING]
        - namespace: company.team
          flowId: myflow2
          states: [SUCCESS]
          labels:
            mykey: myvalue
            another: labelvalue
          inputs: # TODO
            myuser: Loic
# executionFilters, query filters
      where:
        - id: flow1_states # operand: AND # default, needs to be specified only if OR
          filters:
            - field: NAMESPACE
              type: STARTS_WITH
              value: company
            - field: FLOW_ID
              type: IN
              values: 
                - flow1
                - flow2
            - field: STATE
              type: IN
              values: [SUCCESS, WARNING, CANCELLED]

        - id: flow2
          operand: AND
          filters:          
            - field: NAMESPACE
              type: EQUAL_TO
              value: company.team
            - field: FLOW_ID
              type: EQUAL_TO
              value: flow2
            - field: STATE
              type: EQUAL_TO
              value: SUCCESS
            - field: EXPRESSION
              type: IS_TRUE
              value: "{{outputs.output.values.variable == 'value'}}"
            # - field: LABEL
            #   type: EQUAL_TO
            #   values:
            #     mylabelkey: value
            #     another: value
            - field: EXPRESSION
              type: IS_TRUE
              value: "{{ labels.mylabelkey == 'mykey' and labels.mylabelvalue == 'value' }}"
tasks:
  - id: hello
    type: io.kestra.plugin.core.log.Log
    message: I'm triggered after two flows!

Optionally, we can rename all other conditions to not use the Condition word at the end + add an alias with the old name.

Big thanks @loicmathieu for all the iterations and work put into this!

@anna-geller anna-geller added area/backend Needs backend code changes enhancement New feature or request labels Nov 8, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Issues Nov 8, 2024
@loicmathieu
Copy link
Member

@anna-geller

Optionally, we can rename all other conditions to not use the Condition word at the end + add an alias with the old name.

You call ;)
Renaming is easy

@anna-geller
Copy link
Member Author

Alright, I'd say let's do it!

Any time I see:

  • io.kestra.plugin.core.condition.DateTimeBetweenCondition
  • io.kestra.plugin.core.condition.DayWeekInMonthCondition
  • io.kestra.plugin.core.condition.HasRetryAttemptCondition

leads me to believe that this below is way easier to read/grasp:

  • io.kestra.plugin.core.condition.DateTimeBetween
  • io.kestra.plugin.core.condition.DayWeekInMonth
  • io.kestra.plugin.core.condition.HasRetryAttempt

@anna-geller anna-geller changed the title Rename new conditions to ExecutionsWindow and FilteredExecutionsWindow with timeWindow property Refactor new Flow trigger conditions to use a new Flow trigger property preconditions with flows, where and timeWindow subproperties Nov 12, 2024
loicmathieu added a commit that referenced this issue Nov 13, 2024
loicmathieu added a commit that referenced this issue Nov 13, 2024
@loicmathieu loicmathieu linked a pull request Nov 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend Needs backend code changes enhancement New feature or request
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

2 participants