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.

Sign up for GitHub

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

Jump to bottom

Implement Map state #184

Merged
merged 11 commits into from
Oct 4, 2024
Merged

Implement Map state #184

merged 11 commits into from
Oct 4, 2024

Conversation

agrare
Copy link
Member

@agrare agrare commented May 20, 2024 •
edited
Loading

Very WIP, right now just testing payload validation and input processing Just a little WIP now

Depends on:

TODO:

  • ItemProcessor and Workflow should share a common base class
  • Handle running the ItemProcessor "sub-workflow" synchronously
  • Run the ItemProcessor asynchronously
  • Async not always detecting Map is complete and hangs
  • Allow Runner context in ItemProcessor to work with Workflow.wait
  • Be able to run ItemProcessor concurrently (follow-up?)

@agrare agrare requested a review from Fryguy as a code owner May 20, 2024 15:42
@miq-bot miq-bot added the wip label May 20, 2024
@agrare agrare force-pushed the add_map_state branch 5 times, most recently from 69e0eed to 9e773a1 Compare May 20, 2024 16:57
Fryguy
Fryguy reviewed May 20, 2024
View reviewed changes
lib/floe/workflow/item_processor.rb

module Floe
class Workflow
class ItemProcessor < Floe::WorkflowBase
Copy link
Member

Choose a reason for hiding this comment

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

Very cool 👍... Was wondering how you were going to implement it, since it's like a sub workflow

This was referenced Jun 12, 2024
Closed
Merged
@agrare agrare force-pushed the add_map_state branch 2 times, most recently from 6a550cd to da41408 Compare June 13, 2024 20:33
@agrare agrare force-pushed the add_map_state branch 6 times, most recently from f1f9fba to b495db2 Compare June 19, 2024 18:49
@agrare agrare mentioned this pull request Jun 20, 2024
Closed
1 task
@agrare agrare force-pushed the add_map_state branch 5 times, most recently from 7381a42 to ae1edf1 Compare June 25, 2024 16:51
@kbrock
Copy link
Member

kbrock commented Jul 12, 2024

Do we want to leave the ItemProcessorContext in the state (and the history) or do we want to clean that up when this state is finished?

@kbrock
Copy link
Member

kbrock commented Jul 12, 2024

Just saw this: The loop thing I pointed to above gives us concurrent requests.

@kbrock
Copy link
Member

kbrock commented Sep 3, 2024

When you have an itch for floe...

Could you swap the first commit with the second commit?
I'd like to get that into our code base.
The others look like more of a WIP, but that one looks very solid.

e.g.: master...kbrock:floe:add_map_state

When I did this I lost attribution (otherwise I'd put in a PR for the first commit)

Only commits 1 and 2 changed, and they did not change functionally.

possible changes:

  1. I like name staying in the state

@kbrock kbrock mentioned this pull request Sep 19, 2024
Closed
@agrare agrare mentioned this pull request Sep 20, 2024
Merged
@agrare agrare force-pushed the add_map_state branch 2 times, most recently from ec0bb60 to 000632b Compare September 20, 2024 14:22
@agrare agrare changed the title [WIP] Implement Map state Implement Map state Oct 4, 2024
@agrare
Copy link
Member Author

agrare commented Oct 4, 2024

@kbrock okay I've tested this with a Task in the sub-workflow to verify the event handling works for containers run from the child workflows and it was successful, taking out of WIP
This currently only runs one at a time for simplicity we can add parallel in the future

@miq-bot
Copy link
Member

miq-bot commented Oct 4, 2024

Checked commits agrare/floe@6318b74~...1ca1251 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
7 files checked, 0 offenses detected
Everything looks fine. 🍪

@kbrock kbrock added enhancement New feature or request and removed wip labels Oct 4, 2024
@kbrock kbrock merged commit 56737fb into ManageIQ:master Oct 4, 2024
4 of 5 checks passed
@agrare agrare deleted the add_map_state branch October 4, 2024 22:20
agrare added a commit that referenced this pull request Oct 28, 2024
@agrare
Release v0.15.0
ba16eb9 
Added
- Add WorkflowBase base class for Workflow (#279)
- Add tool for using the aws stepfunctions simulator (#244)
- Implement Map state (#184)
- Add Map State Tolerated Failure (#282)
- Run Map iterations in parallel up to MaxConcurrency (#283)
- Implement Parallel State (#291)

Changed
- More granular compare_key and determine path at initialization time (#274)
- For Choice validation, use instance variables and not payload (#277)
- Return ExceedToleratedFailureThreshold if ToleratedFailureCount/Percentage is present (#285)

Fixed
- Fix case on log messages (#280)
- Handle either ToleratedFailureCount or ToleratedFailurePercentage (#284)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fryguy Fryguy Fryguy left review comments

@kbrock kbrock kbrock left review comments

Assignees

@kbrock kbrock

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@agrare @kbrock @miq-bot @Fryguy