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

Add BiConsumer to spring cloud stream plugin #1077

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

LeovR
Copy link
Contributor

@LeovR LeovR commented Nov 17, 2024

This addresses #1071 but only adds the BiConsumer.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to Springwolf. Thanks a lot for creating your first pull request. Please check out our contributors guide and feel free to join us on discord.

Copy link

netlify bot commented Nov 17, 2024

Deploy Preview for springwolf-ui ready!

Name Link
🔨 Latest commit 5d6eb3f
🔍 Latest deploy log https://app.netlify.com/sites/springwolf-ui/deploys/673b68f61724d1000865b2d8
😎 Deploy Preview https://deploy-preview-1077--springwolf-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@LeovR
Copy link
Contributor Author

LeovR commented Nov 17, 2024

I haven't yet used the BiFunction myself. Thus, I only added the BiConsumer, since I'm familiar with that.
I think I should be possible in the same manner.
I also tested the refactoring with the FunctionCatalog here.
But this is kind of a bigger refactoring and does not play well with the configured base-packages.

@timonback
Copy link
Member

timonback commented Nov 18, 2024

The change looks good to me, even when headers are not supported yet, it is a step in the right direction.
Agreed, BiFunction can be a future improvement

Have a look at the publishing functionality, if there is no cloudstream way available, then lets leave it (+ useFqn) unchanged.

@LeovR
Copy link
Contributor Author

LeovR commented Nov 18, 2024

@timonback one could use the StreamBridge for publishing in the cloud stream context.
See e. g. here

But I'm not completely sure whether this works because the other direction is required.

@timonback
Copy link
Member

timonback commented Nov 18, 2024

I also tested the refactoring with the FunctionCatalog here.

I like the change, it seems to be cloud-stream way.
Feel free to go ahead - in a seperate PR.

Regarding the base-package: I think it is fair to assume that the Function must be declared in a specific package (which I hope can be inferred through the type). (Assuming: The information where the Function is defined as a bean is lost)

(Is a dependency required? I am not able to autowire it in the cloudstream-example.)

@timonback
Copy link
Member

@timonback one could use the StreamBridge for publishing in the cloud stream context. See e. g. here

Looks like an interesting starting point to browse the docs. Could be a future enhancement.

Copy link
Member

@timonback timonback left a comment

Choose a reason for hiding this comment

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

Thanks for the update @LeovR. I've run the pipelines, can you check the tests?

@timonback timonback merged commit 972784f into springwolf:master Nov 18, 2024
25 checks passed
@LeovR LeovR deleted the spring-cloud-stream-biconsumer branch November 18, 2024 16:32
ruskaof pushed a commit to ruskaof/springwolf-core that referenced this pull request Nov 20, 2024
* Add BiConsumer to spring cloud stream plugin
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