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

Update CLI docs #346

Merged
merged 17 commits into from
Jul 10, 2023
Merged

Update CLI docs #346

merged 17 commits into from
Jul 10, 2023

Conversation

mtuchi
Copy link
Contributor

@mtuchi mtuchi commented May 9, 2023

Description

Update CLI developer challenge to use workflow execution plan feature, Fixes #339

@mtuchi mtuchi force-pushed the 339-state-from-1-job-to-another branch from 3d0bf5a to 4c49c31 Compare May 19, 2023 07:27
@mtuchi mtuchi requested review from taylordowns2000 and daissatou2 and removed request for taylordowns2000 May 19, 2023 07:28
@mtuchi mtuchi marked this pull request as ready for review May 23, 2023 06:49
Copy link
Contributor

@daissatou2 daissatou2 left a comment

Choose a reason for hiding this comment

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

This is a great addition to the CLI page! I know the OS4H team struggled with the concept of running flow jobs locally. Some feedback to consider:

  1. Please specify if job id is something that should be made up. Also, should it just be unique to that specific workflow plan file?
  2. Can you name the id for the starting job in your example workflow something other than "start"? That confused me a bit.
  3. Can we give an example of a realistic workflow plan? Maybe getUsers and then postUsers? With the current docs, I don’t understand how I could format the execution plan so that the output of getUsers could automatically be used as the input of postUsers. Is it not possible to use a file name as the data?
  4. What is the difference between “execution plan”, “workflow plan” and workflow execution plan”? I’m seeing all three seemingly referring to the same thing. I think it would be a good idea to use one term.
  5. Can you add the text in italics to the docs? “Execution plan is a powerful feature of @openfn/cli that allows you to define a list of jobs and rules for executing them. You can use an Execution Plan to orchestrate the flow of data between systems, and to handle errors and retries in a structured and automated way. For example, if you have two jobs in your workflow (GET users from system A & POST users to system B), you can use your execution plan to automatically run both jobs from start to finish. This imitates the flow and fail trigger patterns on the OpenFn platform where a second job should run after the first one succeeds or fails, respectively, using the data returned from the first job.
  6. After the code snippet on executing the workflow plan (openfn workflow.json -i), include a sentence of what would happen upon execution. For instance “On execution, this workflow plan will first run x job using x data, then run x job using x data”

@mtuchi mtuchi changed the title Update CLI docs Update CLI docs Jun 13, 2023
@taylordowns2000 taylordowns2000 removed their request for review June 13, 2023 21:21
@taylordowns2000
Copy link
Member

@mtuchi , please request a review from me when you've addressed @daissatou2 's changes! excited to get these docs up :-)

@mtuchi mtuchi force-pushed the 339-state-from-1-job-to-another branch from ca94c16 to 246495e Compare June 14, 2023 06:18
@mtuchi
Copy link
Contributor Author

mtuchi commented Jun 14, 2023

@daissatou2 thank you for the feedback 👌🏽, i have address you comments and fine turned the language a bit, Also i have added the examples to show how you can run the workflow. Please give it another spin and let me know if it's still confusing or if we can make it better

@mtuchi mtuchi requested a review from daissatou2 June 14, 2023 15:13
@daissatou2
Copy link
Contributor

@mtuchi I've added some time on your calendar for us to discuss my feedback.

Copy link
Contributor

@daissatou2 daissatou2 left a comment

Choose a reason for hiding this comment

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

discussed feedback on a call with Mtuchi.

Copy link
Member

@taylordowns2000 taylordowns2000 left a comment

Choose a reason for hiding this comment

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

approved from my end. when aicha approves the content we're good to go

@mtuchi
Copy link
Contributor Author

mtuchi commented Jun 27, 2023

discussed feedback on a call with Mtuchi.

@daissatou2 i have incorporated the changes we discussed on our last call, please review and approve this PR if your happy with the changes

@taylordowns2000 taylordowns2000 merged commit 8ea342b into main Jul 10, 2023
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.

Add documentation on how to pass state from 1 job to another when working locally vs on platform
3 participants