You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/07/20 20:37:44 UTC

[GitHub] [beam] nlu90 opened a new pull request, #22373: remove the pulsar admin from pulsar connector

nlu90 opened a new pull request, #22373:
URL: https://github.com/apache/beam/pull/22373

   **Please** add a meaningful description for your change here
   
   Connectors should avoid using Pulsar Admin API as much as possible due to permission control issues.
   
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] johnjcasey commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1231696524

   Closing as we haven't seen updates 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1190757058

   Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @lukecwik for label java.
   R: @chamikaramj for label io.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1221303934

   Reminder, please take a look at this pr: @kennknowles @johnjcasey 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1198061354

   Reminder, please take a look at this pr: @lukecwik @chamikaramj 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] chamikaramj commented on a diff in pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on code in PR #22373:
URL: https://github.com/apache/beam/pull/22373#discussion_r936126507


##########
sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/PulsarIO.java:
##########
@@ -87,10 +83,6 @@ abstract Builder setExtractOutputTimestampFn(
       abstract Read build();
     }
 
-    public Read withAdminUrl(String url) {

Review Comment:
   I don't think we can remove this due to backwards compatibility (seems like this connector not marked experimental).
   
   Can this be addressed by deprecating ? 



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] johnjcasey commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1231696831

   If you want this re-opened, just let us know


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] johnjcasey commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1222397243

   @nlu90 are you still working on Luke's requested changes?


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1213048532

   Reminder, please take a look at this pr: @kennknowles @johnjcasey 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1202409525

   Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
   
   R: @kennknowles for label java.
   R: @johnjcasey for label io.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] lukecwik commented on a diff in pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
lukecwik commented on code in PR #22373:
URL: https://github.com/apache/beam/pull/22373#discussion_r938015066


##########
sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/ReadFromPulsarDoFn.java:
##########
@@ -207,7 +189,7 @@ public OffsetRangeTracker restrictionTracker(
     }
 
     PulsarLatestOffsetEstimator offsetEstimator =
-        new PulsarLatestOffsetEstimator(this.admin, pulsarSource.getTopic());
+        new PulsarLatestOffsetEstimator(null /*this.admin*/, pulsarSource.getTopic());

Review Comment:
   passing in `null` here will cause an NPE when we try to access the `client` within the `PulsarLatestOffsetEstimator`



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] johnjcasey closed pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
johnjcasey closed pull request #22373: remove the pulsar admin from pulsar connector
URL: https://github.com/apache/beam/pull/22373


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] johnjcasey commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
johnjcasey commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1213156973

   run java precommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] lukecwik commented on a diff in pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
lukecwik commented on code in PR #22373:
URL: https://github.com/apache/beam/pull/22373#discussion_r938008308


##########
sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/PulsarIO.java:
##########
@@ -87,10 +83,6 @@ abstract Builder setExtractOutputTimestampFn(
       abstract Read build();
     }
 
-    public Read withAdminUrl(String url) {

Review Comment:
   +1 for deprecating the APIs saying that they are now unused.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] github-actions[bot] commented on pull request #22373: remove the pulsar admin from pulsar connector

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22373:
URL: https://github.com/apache/beam/pull/22373#issuecomment-1231584637

   Reminder, please take a look at this pr: @kennknowles @johnjcasey 


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org