-
Notifications
You must be signed in to change notification settings - Fork 131
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
feat: running context model for test workflows #5633
Conversation
Signed-off-by: Vladislav Sukhin <[email protected]>
✅ Deploy Preview for testkube-docs-preview canceled.
|
Signed-off-by: Vladislav Sukhin <[email protected]>
@@ -7874,6 +7875,9 @@ components: | |||
type: boolean | |||
description: whether webhooks on the executions of this test workflow are disabled | |||
default: false | |||
runningContext: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest designing a new replacement for the existing Running Context, as it's very limiting - having only type and context (= i.e. execution name/ID) string is limiting a lot, as we can't i.e. build the proper tree, or even build URLs (as when there is execution name, you cannot deduct workflow name from it).
Additionally, take in account that we would like to have multiple context information for auditing likely, i.e. User who has scheduled it.
Such solution shouldn't be designed after, but instead - otherwise we will hit either hard limitations, or lack of backwards compatibility, which will result in broken data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, do we have a list of required fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of any solid research conducted on this topic, but we rather don't have anything like that yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @jmorante-ks need some imput about what is required to store besides what we do now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm agree that it would be better do design this first from UX PoV.
@fivenp @TheBrunoLopes any thoughts?
Currently our "context" is more about OSS. I think for now we have only one customer requirements about this:
We know at least that some customers are missing here:
- they want to know who run the test - what means that it'll be the user or some API token, but we have also events taken from triggers.
Signed-off-by: Vladislav Sukhin <[email protected]>
…feature/test-workflow-running-context
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, I would suggest starting from defining the problem and solution architecture, to define what we need (what are expectations, how to achieve that, possible stages) - and then proceeding to implementation.
It doesn't look good - it's likely missing information about the User for auditing purposes, and more information about the reason it has been triggered. Yet, it will be basically impossible to fix it in the future, whatever we will do - as backward compatibility will be pretty hard. Because of that, it's much better to do it carefully and properly.
properties: | ||
interface: | ||
$ref: "#/components/schemas/TestWorkflowRunningContextInterface" | ||
actor: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actor should rather have detailed information about actor.
api/v1/testkube.yaml
Outdated
$ref: "#/components/schemas/TestWorkflowRunningContextInterface" | ||
actor: | ||
$ref: "#/components/schemas/TestWorkflowRunningContextActor" | ||
caller: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the caller idea here
ok, stopping here till final requirements from @jmorante-ks and @TheBrunoLopes |
Signed-off-by: Vladislav Sukhin <[email protected]> # Conflicts: # api/v1/testkube.yaml # cmd/kubectl-testkube/commands/testworkflows/renderer/testworkflowexecution_obj.go # cmd/kubectl-testkube/commands/testworkflows/run.go # cmd/tcl/testworkflow-toolkit/commands/execute.go # cmd/tcl/testworkflow-toolkit/spawn/utils.go # cmd/testworkflow-toolkit/env/config.go # docs/docs/articles/webhooks.mdx # go.mod # go.sum # pkg/api/v1/testkube/model_test_workflow_execution.go # pkg/api/v1/testkube/model_test_workflow_execution_request.go # pkg/api/v1/testkube/model_test_workflow_execution_summary.go # pkg/mapper/testworkflows/openapi_kube.go # pkg/testworkflows/testworkflowexecutor/executor.go # pkg/testworkflows/testworkflowprocessor/stage/container.go
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Signed-off-by: Vladislav Sukhin <[email protected]>
Pull request description
Checklist (choose whats happened)
Breaking changes
Changes
Fixes