[BEAM-1415] PubsubIO style guide fixes, part 1#2750
Conversation
jkff
force-pushed
the
pubsub-style-pt1
branch
3 times, most recently
from
48ab078 to
892feaa
Compare
| public static <T extends Message> Read<T> readProtos(Class<T> messageClass) { | ||
| return PubsubIO.<T>read().withCoder(ProtoCoder.of(messageClass)); | ||
| } | ||
|
|
| return toBuilder() | ||
| .setSubscriptionProvider( | ||
| NestedValueProvider.of(subscription, new SubscriptionTranslator())) | ||
| /* reset topic to null */ |
There was a problem hiding this comment.
I wonder if it's better to just fail if someone calls both fromSubscription and fromTopic, instead of current behavior of last one wins.
| return new Read<>( | ||
| name, subscription, topic, timestampLabel, coder, idLabel, | ||
| parseFn); | ||
| public Read<T> withTimestampLabel(String timestampLabel) { |
There was a problem hiding this comment.
"label" is a remnant of the old beta PubSub API. I believe PubSub now calls these attributes instead - maybe we should rename here to be withTimestampAttribute? (and withIdAttribute)
|
OK, I manually confirmed (via Dataflow jobs run against a test project) that this works fine both with and without attributes. |
|
retest this please |
| } | ||
| } | ||
| if (overriddenTransform.getTimestampLabel() != null) { | ||
| if (overriddenTransform.getTimestampAttribute() != null) { |
There was a problem hiding this comment.
Please make sure doc writers know about s/label/attribute/
There was a problem hiding this comment.
Yup, I'll coordinate with @melap on what to do about all these style-guide-related API changes next week.
|
lgtm |
4 participants
This is just renames, syntax, and AutoValue. Part 2 will be more dramatic (getting rid of Coder in both read and write).
R: @reuvenlax