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

Support editing of certain workflow fields on a provisioned workflow #757

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Jul 4, 2024

Description

Allows passing the parameter update_fields=true on the REST path when updating a template (PUT). This will update the existing template with only the included fields.

Issues Resolved

Resolves #749

Documentation

Please review:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions github-actions bot added the backport 2.x backport PRs to 2.x branch label Jul 4, 2024
Copy link

codecov bot commented Jul 4, 2024

Codecov Report

Attention: Patch coverage is 81.39535% with 16 lines in your changes missing coverage. Please review.

Project coverage is 74.61%. Comparing base (cf1016f) to head (8685442).
Report is 8 commits behind head on main.

Files Patch % Lines
...a/org/opensearch/flowframework/model/Template.java 73.33% 3 Missing and 5 partials ⚠️
...ework/transport/CreateWorkflowTransportAction.java 72.41% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #757      +/-   ##
============================================
+ Coverage     74.37%   74.61%   +0.24%     
- Complexity      745      783      +38     
============================================
  Files            84       84              
  Lines          3825     3873      +48     
  Branches        333      356      +23     
============================================
+ Hits           2845     2890      +45     
  Misses          825      825              
- Partials        155      158       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbwiddis
Copy link
Member Author

dbwiddis commented Jul 4, 2024

POST localhost:9200/_plugins/_flow_framework/workflow?provision=true
{
  "name": "test",
  "description": "Flow template",
  "use_case": "TESTING",
  "version": {
    "template": "1.0.0",
    "compatibility": ["2.12.0", "3.0.0"]
  },
  "workflows": {
    "provision": {
      "user_params": {},
      "nodes": [
        {
          "id": "no op",
          "type": "noop"
        }
      ]
    }
  }
}
{
    "workflow_id": "yl5IfJABgzSGdpsYUKJE"
}
GET localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE
{
    "name": "test",
    "description": "Flow template",
    "use_case": "TESTING",
    "version": {
        "template": "1.0.0",
        "compatibility": [
            "2.12.0",
            "3.0.0"
        ]
    },
    "workflows": {
        "provision": {
            "user_params": {},
            "nodes": [
                {
                    "id": "no op",
                    "type": "noop",
                    "previous_node_inputs": {},
                    "user_inputs": {}
                }
            ],
            "edges": []
        }
    },
    "created_time": 1720072032206,
    "last_updated_time": 1720072032206,
    "last_provisioned_time": 1720072032727
}
GET localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE/_status
{
    "workflow_id": "yl5IfJABgzSGdpsYUKJE",
    "state": "COMPLETED"
}
PUT localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE
{
  "name": "a test with no name",
  "description": "Updated flow template",
  "use_case": "TESTING",
  "version": {
    "template": "1.0.0",
    "compatibility": ["2.12.0", "3.0.0"]
  },
  "workflows": {
    "provision": {
      "user_params": {},
      "nodes": [
        {
          "id": "no op",
          "type": "noop"
        }
      ]
    }
  }
}
400 Bad Request
{
    "error": "The template can not be updated unless its provisioning state is NOT_STARTED: yl5IfJABgzSGdpsYUKJE. Deprovision the workflow to reset the state."
}
PUT localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE?update_fields=true
{
  "workflows": {
    "provision": {
      "user_params": {},
      "nodes": [
        {
          "id": "no op",
          "type": "noop"
        }
      ]
    }
  }
}
400 Bad Request
{
    "error": "You can not update the field [workflows] without updating the whole template."
}
PUT localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE?update_fields=true
{
  "name": "a test with no name",
  "description": "Updated flow template"
}
{
    "workflow_id": "yl5IfJABgzSGdpsYUKJE"
}
GET localhost:9200/_plugins/_flow_framework/workflow/yl5IfJABgzSGdpsYUKJE
{
    "name": "a test with no name",
    "description": "Updated flow template",
    "use_case": "TESTING",
    "version": {
        "template": "1.0.0",
        "compatibility": [
            "2.12.0",
            "3.0.0"
        ]
    },
    "workflows": {
        "provision": {
            "user_params": {},
            "nodes": [
                {
                    "id": "no op",
                    "type": "noop",
                    "previous_node_inputs": {},
                    "user_inputs": {}
                }
            ],
            "edges": []
        }
    },
    "created_time": 1720072032206,
    "last_updated_time": 1720077009607,
    "last_provisioned_time": 1720072032727
}

@dbwiddis
Copy link
Member Author

dbwiddis commented Jul 4, 2024

Signed-off-by: Daniel Widdis <[email protected]>
Copy link
Member

@ohltyler ohltyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for adding this. The allowlisted fields should be sufficient for the frontend.

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall with minor suggestions

Signed-off-by: Daniel Widdis <[email protected]>
Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments. LGTM!

@dbwiddis
Copy link
Member Author

Tests failing on WIndows because clock resolution isn't sufficient to guarantee "instant" increments. :|

@dbwiddis dbwiddis merged commit 7d45f92 into opensearch-project:main Jul 10, 2024
19 of 20 checks passed
@dbwiddis dbwiddis deleted the update-by-field branch July 10, 2024 01:15
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 10, 2024
…757)

* Support editing of certain workflow fields on a provisioned workflow

Signed-off-by: Daniel Widdis <[email protected]>

* Add integ test

Signed-off-by: Daniel Widdis <[email protected]>

* Address review comments

Signed-off-by: Daniel Widdis <[email protected]>

* Refactor field update method to Template class

Signed-off-by: Daniel Widdis <[email protected]>

* Update tests to ensure update timestamp increments

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit 7d45f92)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis pushed a commit that referenced this pull request Jul 10, 2024
…ioned workflow (#775)

Support editing of certain workflow fields on a provisioned workflow (#757)

* Support editing of certain workflow fields on a provisioned workflow



* Add integ test



* Address review comments



* Refactor field update method to Template class



* Update tests to ensure update timestamp increments



---------


(cherry picked from commit 7d45f92)

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport PRs to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Support editing of certain workflow fields on a provisioned workflow
3 participants