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

docs: add parallel step samples #126

Merged
merged 3 commits into from
May 31, 2022
Merged

docs: add parallel step samples #126

merged 3 commits into from
May 31, 2022

Conversation

camiekim
Copy link
Contributor

Per b/232513082

@camiekim camiekim requested a review from a team as a code owner May 26, 2022 12:45
@product-auto-label product-auto-label bot added api: workflows Issues related to the GoogleCloudPlatform/workflows-samples API. samples Issues that are directly related to samples. labels May 26, 2022
- translated: ["", "", ""]
- parallel_translate:
parallel:
shared: [translated] # to write to this variable, we must share it
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Some files have this comment here and some above on line 32. Can you standardize it to be in one location

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, ack.

Comment on lines +35 to +36
# - input: 5 # error, writing to a parent
# variable not marked as shared
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand these comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent is to demonstrate a variable's scope and when an assignment would result in an error. Here's the paragraph that appears before the sample:

"This workflow demonstrates the scope of a shared variable, my_result, as well as variables that are local to their respective branch scopes. Assigning to a variable from a parent scope (input) in a parallel step will result in a deployment error unless the variable is shared in the parallel step."

call: http.get
args:
url: ${"http://mystore.com/getStock/" + input[0]}
result: r # r is local to this branch
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: the variable name r is not very descriptive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. I'll change it and standardize the comments prior to GA.

Comment on lines +33 to +35
- my_result["getStock"]: r.body.some.entry # ok, shared scope
- temp: 1 # ok, local to branch scope
# - input: 5 # error, writing to a parent
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does "OK" mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my reply to your prev. review comment ("ok" is supposed to signal a valid assignment vs "error").

Copy link
Collaborator

@averikitsch averikitsch left a comment

Choose a reason for hiding this comment

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

Heads up any "nits" are optional

@averikitsch averikitsch merged commit 7fdd7a7 into GoogleCloudPlatform:main May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: workflows Issues related to the GoogleCloudPlatform/workflows-samples API. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants