[BEAM-454] Validate Pubsub Topic exists when reading and writing#2989
[BEAM-454] Validate Pubsub Topic exists when reading and writing#2989hepeknet wants to merge 1 commit intoapache:masterfrom
Conversation
|
R: @tgroh |
tgroh
left a comment
tgroh
left a comment
There was a problem hiding this comment.
For a non-template Pipeline, the ValueProvider should have the option associated with it at the time validate is called (I don't believe it currently does, but should). For a templated Pipeline, there may not be an accessible value, which means that we may also wish to gate the validation calls on isAccessible as well. I think we'll properly fail at validation time for more pipelines if that's done, even if we still let some through that end up failing at runtime.
I have a few additional changes in the meantime.
| } catch (IOException ie) { | ||
| throw new RuntimeException("Was not able to validate options: ", ie); | ||
| } finally { | ||
| pubsubClient.close(); |
There was a problem hiding this comment.
Closing the client here seems improper.
| + " execution. If the %1$s is created by an earlier stage of the pipeline, this" | ||
| + " validation can be disabled using #withoutValidation."; | ||
|
|
||
| static void validateTopicExists(PubsubClient pubsubClient, String projectId, String topicName) |
There was a problem hiding this comment.
I don't think this is a hugely useful wrapper, given that we can just call the underlying method and have a precise (based on call location) message
| if (topicName == null) { | ||
| throw new IllegalArgumentException("topicName must not be null"); | ||
| } | ||
| boolean topicExists = listTopics(projectPathFromId(projectId)) |
There was a problem hiding this comment.
This may be expensive; I believe it would be better to use the getTopic API exposed by the PubSub publisher interface (ref. https://cloud.google.com/pubsub/docs/reference/rpc/google.pubsub.v1#google.pubsub.v1.Publisher.GetTopic)
|
Any updates on this PR perhaps? |
6 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-454>] Description of pull requestmvn clean verify.<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
@francesperry does this look ok? Let me know