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 for binding workflow attributes to config element values #263

Conversation

ynedelchev
Copy link
Contributor

@ynedelchev ynedelchev commented Apr 11, 2024

Description

This Pull Request is enabling the following feature for Aria Automation Orchestrator Workflows

Be able to bind attribute values to Configuration Element variables

To do that one have to provide the following annotation for the workflow ts file (SomeFile.wf.ts)

@Workflow({
        id: "<Some Id>",
        name: "<Some Name>",
        path: "<Some Path>",
        attributes: {
            attributeName: {
            type: "string",
            bind: true,   
            value: "Some/Path/To/ConfigurationElement/variableName"
            }
        }
    })

Here
bind: true - means that we have to bind the value of the attribute to Configuration Element variable.
value: "Some/Path/To/ConfigurationElement/variableName" - points to the Configuration Element and variable inside the Configuration Element to bind to.

Checklist

@ynedelchev ynedelchev requested a review from a team as a code owner April 11, 2024 07:44
@VenelinBakalov
Copy link
Collaborator

Could you update CHANGELOG.md and Release.md

@ynedelchev
Copy link
Contributor Author

Could you update CHANGELOG.md and Release.md
Done

@dimitar-nikolov-lazarov

This PR looks like containing changes from https://github.com/vmware/build-tools-for-vmware-aria/pull/261/commits
It is harder to determine only its changes.
It will be necessary to merge them in proper order :)

typescript/vrotsc/src/types.ts Outdated Show resolved Hide resolved
typescript/vrotsc/src/compiler/program.ts Show resolved Hide resolved
if (id == null) {
throw new Error(`Invalid syntax for attribute "${att.name}" in workflow "${workflow.name}". It is specified that its value is bound to configuration element with path "${path}" and variable "${key}"`
+ `, but a configuration element with path "${path}" cannot be found in project at that stage. If you belive it is indeed really part of the project, `
+ `please try moving the file earlier alphabetically so it is processed earlier than the workflow that uses it. Currently available configuration elements are: `

Choose a reason for hiding this comment

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

Good catch. Maybe is good idea to have feature task to extend processing of the workflows to be after configurations and resource elements - to avoid such errors.

@ynedelchev
Copy link
Contributor Author

This PR looks like containing changes from https://github.com/vmware/build-tools-for-vmware-aria/pull/261/commits It is harder to determine only its changes. It will be necessary to merge them in proper order :)

Fair comment. I do not know how that slipped.
Still it turned out to be not so easy for fix/revert as if I revert the files to their state in main branch, then the super-linter starts to complain about JavaDoc issues.
Do you see that as a blocker ?
I think if the other Pull Request is approved and merged, then it would be just formal merge from main, so we can proceed with just adjusting the order of merging as you suggest.

…ents.

This is done by providing the following annotation:
```
@workflow({
    id: "<Some Id>",
    name: "<Some Name>",
    path: "<Some Path>",
    attributes: {
        attributeName: {
        type: "string",
        bind: true,
        value: "Some/Path/To/ConfigurationElement/variableName"
        }
    }
})
```
Please note that for the attribute, we have type, bind, and value.
bind is a boolean showing if the attribute is bound to a configuration element.
If so, then value shows the path to the ConfigurationElement and the variable inside it to bind to,
so in the forkflow the attribute value would equal to the value specified in the ConfigurationElement variable.

Signed-off-by: Yordan Nedelchev <ynedelchev@vmware.com>
Signed-off-by: Yordan Nedelchev <ynedelchev@vmware.com>
Signed-off-by: Yordan Nedelchev <ynedelchev@vmware.com>
dimitar-nikolov-lazarov Apr 12, 2024

Same as type definitions i other file.
It would be better to specify exactly that the type is dictionary of key: string, any/string(value).
{ [key: string]: any } or { [key: string]: string }

Signed-off-by: Yordan Nedelchev <ynedelchev@vmware.com>
@ynedelchev ynedelchev force-pushed the support-for-binding-workflow-attributes-to-config-element-values branch from befb2a9 to c4bd92d Compare April 18, 2024 08:23
@ynedelchev
Copy link
Contributor Author

This PR looks like containing changes from https://github.com/vmware/build-tools-for-vmware-aria/pull/261/commits It is harder to determine only its changes. It will be necessary to merge them in proper order :)

Fair comment. I do not know how that slipped. Still it turned out to be not so easy for fix/revert as if I revert the files to their state in main branch, then the super-linter starts to complain about JavaDoc issues. Do you see that as a blocker ? I think if the other Pull Request is approved and merged, then it would be just formal merge from main, so we can proceed with just adjusting the order of merging as you suggest.

UPDATE:
rebased and the problematic commit removed.

const key = value.substring(index+1).trim();
const path = value.substring(0, index).trim();
const id = context.configIdsMap[path];
if (id == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the check should be if (id === null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

akantchev

    I think the check should be if (id === null)

ynedelchev
    Fixed :)

Signed-off-by: Yordan Nedelchev <ynedelchev@vmware.com>
@ynedelchev ynedelchev merged commit cd2e857 into main Apr 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants