Skip to content

Create forloop chain in the example pack#3328

Merged
Kami merged 1 commit intoStackStorm:masterfrom
Carles-Figuerola:master
May 23, 2017
Merged

Create forloop chain in the example pack#3328
Kami merged 1 commit intoStackStorm:masterfrom
Carles-Figuerola:master

Conversation

@Carles-Figuerola
Copy link
Contributor

This is an example chain for a job that needs to be done in several steps because the bulk of each of the passes is bottlenecked (in the case I found, arguments between python calls cannot exceed 131072 characters). The "get_data" and the "push_data" actions can be from different packs if need be if they have support for "paging" on their requests.

If this feels valuable I could write something up more detailed as a blogpost to show how each of the steps work and post an example run and why anyone would want to use it. In this case, this is how it looks for now:

forloop_chain

@Kami
Copy link
Member

Kami commented Apr 5, 2017

Thanks for the contribution - will review it shortly.

@Kami
Copy link
Member

Kami commented Apr 5, 2017

From a high level the change looks good to me, but since we also use code from examples pack in various tests it would be better, if possible, that actions in those pack wouldn't perform HTTP requests and only depend on core.local actions.

This way if we ever add tests for those new actions they will be more reliable and won't depend on 3rd party service which could be offline or unavailable because of various issues.

@Kami
Copy link
Member

Kami commented Apr 5, 2017

If this feels valuable I could write something up more detailed as a blogpost to show how each of the steps work and post an example run and why anyone would want to use it.

That would be great. We would be happy to feature a guest post from you about this particular looping use case on our blog.

Feel free to start a draft in google docs / github gist / similar and share it with us once you make some progress and we will work from there :)

@Kami
Copy link
Member

Kami commented Apr 5, 2017

@lakshmi-kannan I'm personally fine with this change, but as mentioned above, I have some concerns with including it in the default examples pack - it depends on external service and parsing HTML which is not so robust (it would break if github changes HTML structure and potentially also fails if github is unavailable / there is a networking issue / similar).

What do others think?

@Kami
Copy link
Member

Kami commented May 12, 2017

/cc @lakshmi-kannan @m4dcoder et. al - would be good to decide on how to proceed with that^

@Kami
Copy link
Member

Kami commented May 23, 2017

I will go ahead and merge this for now, but eventually it would be better if we moved to an example which is less error prone and doesn't depend on external service.

Thanks again.

@Kami Kami merged commit 1ef9c3b into StackStorm:master May 23, 2017
@Kami
Copy link
Member

Kami commented May 23, 2017

Pushes a small style clean-up - #3428

In the future, please use 4 spaces for a tab (some how our automatic lint check didn't catch this).

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.

2 participants