You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/01 16:27:00 UTC
[GitHub] [pulsar] david-streamlio opened a new pull request #9406: Issue-9402: Added ImmediateTriggerer class
david-streamlio opened a new pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406
<!--
### Contribution Checklist
*(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
Fixes #9402
### Motivation
We needed a new type of tirggerer for batch sources that is triggered exactly once, e.g. a S3 source that receives CDC notifications via an SQS topic. In this scenario, the discover phase only needs to run once to setup the SQS connection, etc.
### Modifications
Added a new class called ImmediateTriggerer
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
### Does this pull request potentially affect one of the following parts:
NO
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (yes / no)
- The public API: (yes / no)
- The schema: (yes / no / don't know)
- The default values of configurations: (yes / no)
- The wire protocol: (yes / no)
- The rest endpoints: (yes / no)
- The admin cli options: (yes / no)
- Anything that affects deployment: (yes / no / don't know)
### Documentation
- Does this pull request introduce a new feature? (yes / no) YES
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented). JavaDocs
- If a feature is not applicable for documentation, explain why?
- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-771010962
@david-streamlio would you please help provide some description for this pr?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-782452813
@sijie I don't see any unit test or integration tests for the existing `CronTriggerer` class so I am unsure how to go about testing this class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#discussion_r580858077
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/io/PulsarFunctionE2ETest.java
##########
@@ -266,6 +270,28 @@ void setup(Method method) throws Exception {
fileServerThread = new Thread(() -> {
try {
fileServer = HttpServer.create(new InetSocketAddress(0), 0);
+ fileServer.createContext("/pulsar-io-batch-data-generator.nar", he -> {
+ try {
+
+ Headers headers = he.getResponseHeaders();
+ headers.add("Content-Type", "application/octet-stream");
+
+ File file = new File(getClass().getClassLoader().getResource("pulsar-io-batch-data-generator.nar").getFile());
+ byte[] bytes = new byte [(int)file.length()];
+
+ FileInputStream fileInputStream = new FileInputStream(file);
Review comment:
please close this resource
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-771683157
@sijie An example use case would be an S3 source connector that uses SQS to get event notifications of changes rather than scanning the entire S3 bucket for changes. https://docs.aws.amazon.com/AmazonS3/latest/dev/NotificationHowTo.html
In such a scenario, the discover phase of the source would connect to the SQS queue as described [here](https://docs.aws.amazon.com/AmazonS3/latest/dev/ways-to-add-notification-config-to-bucket.html), and only need to execute once.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on a change in pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#discussion_r581289384
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/io/PulsarFunctionE2ETest.java
##########
@@ -266,6 +270,28 @@ void setup(Method method) throws Exception {
fileServerThread = new Thread(() -> {
try {
fileServer = HttpServer.create(new InetSocketAddress(0), 0);
+ fileServer.createContext("/pulsar-io-batch-data-generator.nar", he -> {
+ try {
+
+ Headers headers = he.getResponseHeaders();
+ headers.add("Content-Type", "application/octet-stream");
+
+ File file = new File(getClass().getClassLoader().getResource("pulsar-io-batch-data-generator.nar").getFile());
+ byte[] bytes = new byte [(int)file.length()];
+
+ FileInputStream fileInputStream = new FileInputStream(file);
Review comment:
I have rebased onto the current master branch and added code to close all the opened input streams in the code
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio closed pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio closed pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-777185222
@david-streamlio is it possible to add an integration test for this trigger?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-771683157
@sijie An example use case would be an S3 source connector that uses SQS to get event notifications of changes rather than scanning the entire S3 bucket for changes. https://docs.aws.amazon.com/AmazonS3/latest/dev/NotificationHowTo.html
In such a scenario, the discover phase of the source would connect to the SQS queue as described [here](https://docs.aws.amazon.com/AmazonS3/latest/dev/ways-to-add-notification-config-to-bucket.html), and only need to execute once.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#issuecomment-786259166
Replacing the https://github.com/apache/pulsar/pull/9727
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] david-streamlio commented on a change in pull request #9406: Issue-9402: Added ImmediateTriggerer class
Posted by GitBox <gi...@apache.org>.
david-streamlio commented on a change in pull request #9406:
URL: https://github.com/apache/pulsar/pull/9406#discussion_r581289384
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/io/PulsarFunctionE2ETest.java
##########
@@ -266,6 +270,28 @@ void setup(Method method) throws Exception {
fileServerThread = new Thread(() -> {
try {
fileServer = HttpServer.create(new InetSocketAddress(0), 0);
+ fileServer.createContext("/pulsar-io-batch-data-generator.nar", he -> {
+ try {
+
+ Headers headers = he.getResponseHeaders();
+ headers.add("Content-Type", "application/octet-stream");
+
+ File file = new File(getClass().getClassLoader().getResource("pulsar-io-batch-data-generator.nar").getFile());
+ byte[] bytes = new byte [(int)file.length()];
+
+ FileInputStream fileInputStream = new FileInputStream(file);
Review comment:
i have rebased onto the current master branch and added code to close all the opened input streams in the code
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org