Skip to content

Enhanced Rest Data Sink Component with ability to add headers to request, retry the request on failure, and upgrade to current streampipes interfaces #3541

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

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

heisenbergs-uncertainty
Copy link

@heisenbergs-uncertainty heisenbergs-uncertainty commented Mar 26, 2025

Purpose

This enhances the currently implemented RestPublisher Data Sink which has very little functionality outside of making a request. Many implementations of REST functionality are going to be utilizing the sink to trigger a webhook which should at bare minimum use authorization to confirm the legitimacy of the request origin. This enhancement allows for the addition of custom headers so that more advanced REST scenarios can be addressed with the RestPublisher. I have also added retry functionality on failure, timeouts, and better logging for more advanced debugging.

This also upgrades the interfaces on from older streampipes models to the newer "IDataSinkInterface" as demonstrated by the Streampipes tutorial documentation.

Here is a picture of what the new sink looks like in the UI:

image

Here is an example of this same request hitting RequestBin:

image

Remarks

No remarks.

PR introduces (a) breaking change(s): no

PR introduces (a) deprecation(s): no

…functionality, and more extensive logging.
@github-actions github-actions bot added java Pull requests that update Java code pipeline elements Relates to pipeline elements backend Everything that is related to the StreamPipes backend labels Mar 26, 2025
@tenthe
Copy link
Contributor

tenthe commented Mar 28, 2025

Hi @heisenbergs-uncertainty,

thanks a lot for your PR! The changes are highly welcome. I really appreciate the update to the new API and the additional functionality for the sink.

Required Changes

There are some changes required to ensure that this PR works and that existing instances of the RestSink are migrated correclty.

  • Checkstyle Issues: There are some checkstyle problems—please check the GitHub Actions logs for details.
  • Version Update: Update the sink version to 1 (see line 74).
  • Migration Script: A migration script is required to update existing sinks in pipelines. You can refer to DataLakeSinkMigrationV2 for guidance.
  • Displayed Labels: Move static property texts to string.en in resource directory org.apache.streampipes.sinks.brokers.jvm.rest. Use the Labels.withId API—see the URL_KEY parameter of the RestSink before the changes for reference.

Please let us know if you need any assistance or if some of the changes I described are not clear.

Suggestion

What do you think about making MAX_RETRIES and RETRY_DELAY_MS user-configurable with default values? This would allow users to adjust them as needed.

Thank you again for your PR and we look forward to working with you.

@heisenbergs-uncertainty
Copy link
Author

Hi @heisenbergs-uncertainty,

thanks a lot for your PR! The changes are highly welcome. I really appreciate the update to the new API and the additional functionality for the sink.

Required Changes

There are some changes required to ensure that this PR works and that existing instances of the RestSink are migrated correclty.

* **Checkstyle Issues**: There are some checkstyle problems—please check the GitHub Actions logs for details.

* **Version Update**: Update the sink version to **1** (see line 74).

* **Migration Script**: A migration script is required to update existing sinks in pipelines. You can refer to [`DataLakeSinkMigrationV2`](https://github.com/apache/streampipes/blob/43573850423475a429f517db6502fbe30d1a25b8/streampipes-extensions/streampipes-sinks-internal-jvm/src/main/java/org/apache/streampipes/sinks/internal/jvm/datalake/migrations/DataLakeSinkMigrationV2.java#L33) for guidance.

* **Displayed Labels**: Move static property texts to `string.en` in resource directory `org.apache.streampipes.sinks.brokers.jvm.rest`. Use the `Labels.withId` API—see the `URL_KEY` parameter of the `RestSink` before the changes for reference.

Please let us know if you need any assistance or if some of the changes I described are not clear.

Suggestion

What do you think about making MAX_RETRIES and RETRY_DELAY_MS user-configurable with default values? This would allow users to adjust them as needed.

Thank you again for your PR and we look forward to working with you.

Thank you for the suggestions! This is helpful to figure out how to clean this up and get the PR passing. I'll get the version update and migration script done asap.

The displayed labels comment makes a lot of sense and answers some of my confusion. I spent a long time looking for why so many components were using "withId" instead of "from". It would be helpful to include this in the "Extending Streampipes" tutorial section on the website. I could have also just missed it while skimming but I did look for a decent period of time and couldn't understand its usage.

In regards to the retry delay, I had this same thought and was planning on adding it later on but I will go ahead and include it now! Makes sense to include this in case anybody doesn't want the retry functionality up front.

@heisenbergs-uncertainty
Copy link
Author

heisenbergs-uncertainty commented Mar 28, 2025

@tenthe I could use help with conditional ui elements and how to add them.

For example, the user input should be required for RETRY_DELAY_MS and NUM_RETRY integers if and only if the user has enabled them via the IS_RETRY_ENABLED boolean (or single selection... whatever suits better)

@dominikriemer
Copy link
Member

Hi @heisenbergs-uncertainty thanks for your contribution.
We currently don't really have a concept for conditional ui elements, but since we've discussed this feature many times in the past, I'll be working on adding such a UI element which you can then also use in the REST sink.
I think it will be a combination of a slider and an element group where arbitrary other ui elements can be included.

@heisenbergs-uncertainty
Copy link
Author

Hi @heisenbergs-uncertainty thanks for your contribution.

We currently don't really have a concept for conditional ui elements, but since we've discussed this feature many times in the past, I'll be working on adding such a UI element which you can then also use in the REST sink.

I think it will be a combination of a slider and an element group where arbitrary other ui elements can be included.

Sounds good. I'll just add documentation for it and maybe submit an issue as a reminder to me to update it to the conditional component once that's complete. Functionality should be done at this point and I just need to update the migration scripts.

Matthew Holden and others added 7 commits April 12, 2025 00:26
… instead of labels.from. This change simplifies the configuration and enhances readability.

# Conflicts:
#	streampipes-extensions/streampipes-sinks-brokers-jvm/src/main/java/org/apache/streampipes/sinks/brokers/jvm/rest/RestSink.java
…ble to input if they want to retry the request and then configure the retry delay and max number of retries.
…type to a slider since I learned that a slider represents true or false in streampipes. I updated integerts from .requiredIntegerParameter to a static property free text integer since these values more closely represent a static property rather than a parameter. I also added the V0->V1 migration for those currently using the RestSink.
…and attempted to format correctly but I couldn't figure out everything in IDEA Ultimate.
@heisenbergs-uncertainty
Copy link
Author

@tenthe @dominikriemer I might need help on the checkstyle issues. I don't typically work in Java and I am not totally certain of IDEA IDE and I am having trouble getting the formatting to work correctly with the checkstyle.xml. It seems like all of my checkstyle errors are currently related to indentations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Everything that is related to the StreamPipes backend java Pull requests that update Java code pipeline elements Relates to pipeline elements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants