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

Add path system to reference nested data in process #1545

Merged
merged 14 commits into from Dec 13, 2019

Conversation

krhubert
Copy link
Contributor

@krhubert krhubert commented Dec 3, 2019

  • small refactor

dep #1540
close #1306

@krhubert krhubert added the enhancement New feature or request label Dec 3, 2019
@krhubert krhubert self-assigned this Dec 3, 2019
@NicolasMahe NicolasMahe added this to the next milestone Dec 4, 2019
@NicolasMahe
Copy link
Member

optional path test added

@NicolasMahe NicolasMahe added this to Review in progress in Sprint week 49 & 50 via automation Dec 6, 2019
@krhubert krhubert changed the title Add data query language in orchestrator Add Path to Reference Output Dec 9, 2019
@krhubert krhubert marked this pull request as ready for review December 9, 2019 09:49
@NicolasMahe NicolasMahe changed the title Add Path to Reference Output Reference nested data with path system in process Dec 13, 2019
@NicolasMahe NicolasMahe added the release:add Pull requests that add something label Dec 13, 2019
@NicolasMahe NicolasMahe changed the title Reference nested data with path system in process Add path system to reference nested data in process Dec 13, 2019
@NicolasMahe NicolasMahe removed the release:add Pull requests that add something label Dec 13, 2019
@NicolasMahe
Copy link
Member

NicolasMahe commented Dec 13, 2019

TODO:

  • add e2e test on nested ref. the e2e tests are testing only 1 level of nested ref.

@@ -259,3 +260,51 @@ func (s *Orchestrator) processTask(nodeKey string, task *process.Process_Node_Ta
}, s.accountName, s.accountPassword)
return err
}

func resolveRef(data *types.Struct, path *process.Process_Node_Map_Output_Reference_Path) (*types.Value, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I find this code really complicated with the v that is assigned in multiple places and the iteration over the path from an existing path. I think this would be much simpler with a recursive function.

@@ -224,9 +225,9 @@ func (s *Orchestrator) resolveInput(wfHash hash.Hash, exec *execution.Execution,
if err != nil {
return nil, err
}
return s.resolveInput(wfHash, parent, nodeKey, outputKey)
return s.resolveInput(wfHash, parent, nodeKey, path)
Copy link
Member

Choose a reason for hiding this comment

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

resolveInput wasn't resolving any path, I just added a commit with the fix

Sprint week 49 & 50 automation moved this from Review in progress to Reviewer approved Dec 13, 2019
@antho1404 antho1404 merged commit 9032e71 into dev Dec 13, 2019
Sprint week 49 & 50 automation moved this from Reviewer approved to Done Dec 13, 2019
@antho1404 antho1404 deleted the feature/reference-data branch December 13, 2019 09:28
@NicolasMahe NicolasMahe mentioned this pull request Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Reference nested data in mapping
3 participants