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

Bug flow_web_service should pass the record onto the next #453

Closed
jaypha opened this issue Aug 5, 2022 · 2 comments
Closed

Bug flow_web_service should pass the record onto the next #453

jaypha opened this issue Aug 5, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@jaypha
Copy link
Contributor

jaypha commented Aug 5, 2022

flow_web_service::execute() returns a bool. As a flow step it should be returning a value, so it can be passed on the next step.
It appears to be written like a connector. Should 'result' perhaps be passed on through the flow iterator rather than set as a variable? (at least in addition to).
It is not identified as either a reader or a writer, even though it appears to be capable of both.

@jaypha jaypha added the bug Something isn't working label Aug 5, 2022
@keevan
Copy link
Contributor

keevan commented Aug 5, 2022

Yes it seems like it would in fact cause issues if any downstreams wanted to use the value.

I do agree that a web service should be available as a connector and as a flow step, and @brendanheywood shares my sentiment as shown #422 and #377 (comment)

In the mean time, I do think the input should flow through as-is if it does not need to be touched.

@brendanheywood
Copy link
Contributor

At the very least it should pass the record it was given on as-is. It definitely should not replace the record with the ws's output nor should it ever return false.

The semantics of a reader and a write are going to be gray for lots of step, I don't really think the labels as a perfect fit so I'm not super worried about those and they are mostly cosmetic anyway.

I can see use cases for both just stashing it somewhere like it is now, and also writing it into the record as an extra attribute somewhere. The middle ground is probably going to be config to say what to do with it, similar to the output mappings. I think we should split this up into 2 issues, and this one should only focus on the iterator return value being passed along the chain. The second part can wait, if we do end up refactoring things so a step can act and either a flow or connector depending on its config and context then that refactor should come first.

@brendanheywood brendanheywood changed the title Some concerns with flow_web_service implementation Bug flow_web_service should pass the record onto the next Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants