tenthe commented on PR #3541: URL: https://github.com/apache/streampipes/pull/3541#issuecomment-2761577824
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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
