Combine create api with provision api by adding a provision param

[jackiehanyang]

Description

Combine create api with provision api by adding a provision=true parameter to create api

Issues Resolved

#259

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.

[amitgalitz]

Do we not have a dry_run optional param at all anymore?

[jackiehanyang]

Do we not have a dry_run optional param at all anymore?

What's the benefit of keeping the dry_run parameter? provision covers all the check dry_run does. Also as this project is not launched yet, this is not a breaking change.

[joshpalis]

What's the benefit of keeping the dry_run parameter? provision covers all the check dry_run does. Also as this project is not launched yet, this is not a breaking change.

Dry run param in the create workflow API should remain unchanged. This is to allow for validation prior to saving into the global context index. Combining the two APIs together should just add a new path rather than supersede existing ones.

Though provision does validate as well, since we're combining the two it would make sense to validate on create (before we index) rather than after. I would suggest we invoke the CreateWorkflowTransportAction with a dry-run flag set to true and then provision via ProvisionWorkflowTransportAction

[jackiehanyang]

What's the benefit of keeping the dry_run parameter? provision covers all the check dry_run does. Also as this project is not launched yet, this is not a breaking change.

Dry run param in the create workflow API should remain unchanged. This is to allow for validation prior to saving into the global context index. Combining the two APIs together should just add a new path rather than supersede existing ones.

Though provision does validate as well, since we're combining the two it would make sense to validate on create (before we index) rather than after. I would suggest we invoke the CreateWorkflowTransportAction with a dry-run flag set to true and then provision via ProvisionWorkflowTransportAction

Are you suggesting when provision=true we treat it like dryrun=true && provision=true?

[joshpalis]

Are you suggesting when provision=true we treat it like dryrun=true && provision=true?

Yes

[dbwiddis]

Are you suggesting when provision=true we treat it like dryrun=true && provision=true?

Yes

I may be misunderstanding dryrun or maybe it's poorly named.

Dry run usually means do validation checks but don't actually index anything. So my thoughts would be:

• dryRun false, provision false = validates and saves to index, does not provision <-- normal 2-step case front-end will use
• dryRun false, provision true = validates and saves to index, then provisions <-- normal use case for a debugged workflow for one-click
• dryRun true, provision false = validates but doesn't save anything <-- kinda like precommit checks :)
• dryRun true, provision true = illegal

Does it mean something else than that?

[joshpalis]

Dry run usually means do validation checks but don't actually index anything.

@dbwiddis currently when we dry-run, if the template passes validation, then we index, if not then we return an error

[jackiehanyang]

Are you suggesting when provision=true we treat it like dryrun=true && provision=true?

Yes

I may be misunderstanding dryrun or maybe it's poorly named.

Dry run usually means do validation checks but don't actually index anything. So my thoughts would be:

• dryRun false, provision false = validates and saves to index, does not provision <-- normal 2-step case front-end will use
• dryRun false, provision true = validates and saves to index, then provisions <-- normal use case for a debugged workflow for one-click
• dryRun true, provision false = validates but doesn't save anything <-- kinda like precommit checks :)
• dryRun true, provision true = illegal

Does it mean something else than that?

My understanding of all the conditions is:

• dryRun=false && provision=false, no formate validation, no provision. Just save the template to index
• dryRun=false && provision=true, save the template to index, then provisioning the template. Provision could failed due to template formatting error like missing certain input files, or could due to template is a cyclic graph. (it just performs all the check that come with the Provision API)
• dryRun=true && provision = false, do formate validation first, only save the template to index if it passes the check.
• dryRun=true && provision = true, do formate validation first, save the template to index if it passes the check, then provisioning the template. Although provision step also checks for formatting issues, which is already covered by dryRun check.

I think no matter how customer mix&match these two parameters, they should all be legal. The dryRun=true && provision = true case helps customer to fail early is there's any formatting issue

PROPOSAL: We retain both the dryRun and provision parameters, allowing customers to mix and match them as they wish.

[joshpalis]

My understanding of all the conditions is:

dryRun=false && provision=false, no formate validation, no provision. Just save the template to index
dryRun=false && provision=true, save the template to index, then provisioning the template. Provision could failed due to template formatting error like missing certain input files, or could due to template is a cyclic graph. (it just performs all the check that come with the Provision API)
dryRun=true && provision = false, do formate validation first, only save the template to index if it passes the check.
dryRun=true && provision = true, do formate validation first, save the template to index if it passes the check, then provisioning the template. Although provision step also checks for formatting issues, which is already covered by dryRun check.
I think no matter how customer mix&match these two parameters, they should all be legal. The dryRun=true && provision = true case helps customer to fail early is there's any formatting issue

PROPOSAL: We retain both the dryRun and provision parameters, allowing customers to mix and match them as they wish.

Agreed with this proposal

[dbwiddis]

My understanding of all the conditions is:
dryRun=false && provision=false, no formate validation, no provision. Just save the template to index
dryRun=false && provision=true, save the template to index, then provisioning the template. Provision could failed due to template formatting error like missing certain input files, or could due to template is a cyclic graph. (it just performs all the check that come with the Provision API)
dryRun=true && provision = false, do formate validation first, only save the template to index if it passes the check.
dryRun=true && provision = true, do formate validation first, save the template to index if it passes the check, then provisioning the template. Although provision step also checks for formatting issues, which is already covered by dryRun check.
I think no matter how customer mix&match these two parameters, they should all be legal. The dryRun=true && provision = true case helps customer to fail early is there's any formatting issue
PROPOSAL: We retain both the dryRun and provision parameters, allowing customers to mix and match them as they wish.

Agreed with this proposal

I'm ok with this except that it doesn't match my understanding of the term "dryRun". For example in AD the "create detector" with dryRun does not actually create a detector (actually it's just a variable but it corresponds to validating).

So if we're going this way let's use "validate" rather than dry run.

https://en.wikipedia.org/wiki/Dry_run_(testing)

[dbwiddis]

Other opensearch use of dryrun: https://opensearch.org/docs/latest/observing-your-data/alerting/api/

You can add the optional ?dryrun=true parameter to the URL to show the results of a run without actions sending any message.

Use of validate: https://opensearch.org/docs/latest/observing-your-data/ad/api/

Returns whether the detector configuration has any issues that might prevent OpenSearch from creating the detector.

You can use the validate detector API operation to identify issues in your detector configuration before creating the detector.

Use of validate: https://opensearch.org/docs/latest/im-plugin/ism/error-prevention/api/

Pass the validate_action=true path parameter in the Explain API URI to see the validation status and message.

[jackiehanyang]

My understanding of all the conditions is:
dryRun=false && provision=false, no formate validation, no provision. Just save the template to index
dryRun=false && provision=true, save the template to index, then provisioning the template. Provision could failed due to template formatting error like missing certain input files, or could due to template is a cyclic graph. (it just performs all the check that come with the Provision API)
dryRun=true && provision = false, do formate validation first, only save the template to index if it passes the check.
dryRun=true && provision = true, do formate validation first, save the template to index if it passes the check, then provisioning the template. Although provision step also checks for formatting issues, which is already covered by dryRun check.
I think no matter how customer mix&match these two parameters, they should all be legal. The dryRun=true && provision = true case helps customer to fail early is there's any formatting issue
PROPOSAL: We retain both the dryRun and provision parameters, allowing customers to mix and match them as they wish.

Agreed with this proposal

I'm ok with this except that it doesn't match my understanding of the term "dryRun". For example in AD the "create detector" with dryRun does not actually create a detector (actually it's just a variable but it corresponds to validating).

So if we're going this way let's use "validate" rather than dry run.

https://en.wikipedia.org/wiki/Dry_run_(testing)

When dryRun is set to true, we proceed with indexing only if it passes the validation check - this is the current design of the create workflow API. If we think it's necessary to revise the design of the create workflow API, I can open a separate issue for this matter.

[dbwiddis]

When dryRun is set to true, we proceed with indexing only if it passes the validation check

I'm fine with this, I'm just quibbling over using the term "dryRun" (used only once in the opensearch API) vs. "validate" used many times and much more clear on what's happening.

[jackiehanyang]

When dryRun is set to true, we proceed with indexing only if it passes the validation check

I'm fine with this, I'm just quibbling over using the term "dryRun" (used only once in the opensearch API) vs. "validate" used many times and much more clear on what's happening.

Let's the continue the discussion in this issue - #291

[amitgalitz]

LGTM, one very minor comment only