Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Google Cloud Pub/Sub ordering key #2863 #2864

Merged
merged 6 commits into from
Apr 29, 2022
Merged

Google Cloud Pub/Sub ordering key #2863 #2864

merged 6 commits into from
Apr 29, 2022

Conversation

LukBed
Copy link
Contributor

@LukBed LukBed commented Apr 21, 2022

@lightbend-cla-validator
Copy link

At least one commit author ([email protected]) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@LukBed LukBed changed the title Google Cloud Pub/Sub Support ordering key for Pub/Sub #2863 Google Cloud Pub/Sub ordering key #2863 Apr 21, 2022
ennru
ennru reviewed Apr 22, 2022
View reviewed changes
google-cloud-pub-sub/src/main/scala/akka/stream/alpakka/googlecloud/pubsub/model.scala

/**
* Java API
*/
def create(data: java.util.Optional[String],
attributes: java.util.Optional[java.util.Map[String, String]],
messageId: String,
publishTime: Instant) =
publishTime: Instant,
orderingKey: java.util.Optional[String]) =
new PubSubMessage(Option(data.orElse(null)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stay source compatible for Java users, you should add an overload for create with the new orderingKey and keep the existing create with its signature. (Just as you did with PublishMessage.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@LukBed LukBed requested a review from ennru April 22, 2022 12:19
@ennru
Copy link
Member

ennru commented Apr 25, 2022

Some Java code got reformatted again. Please run javafmtAll.

@LukBed
Copy link
Contributor Author

LukBed commented Apr 25, 2022

Some Java code got reformatted again. Please run javafmtAll.

Formatted - both Java and Scala

@ennru
Copy link
Member

ennru commented Apr 25, 2022

Our binary compatibility check complains now. https://github.com/akka/alpakka/runs/6154638614?check_suite_focus=true#step:6:530
Some problems it raises might be OK to suppress, but I need to look closer to know which.

@LukBed
Copy link
Contributor Author

LukBed commented Apr 26, 2022

Binary compatibility fixed

@LukBed
Copy link
Contributor Author

LukBed commented Apr 27, 2022

Sorry, test compilation fixed

ennru
ennru approved these changes Apr 29, 2022
View reviewed changes
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for going through all the quirks the binary compatibility promise requires.
LGTM.

@ennru ennru modified the milestone: 4.0.0 Apr 29, 2022
@ennru ennru merged commit f1dbbcb into akka:master Apr 29, 2022
@LukBed LukBed deleted the pub-sub-ordering branch April 29, 2022 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ennru ennru ennru approved these changes

Assignees
No one assigned
Labels
p:google-cloud-pub-sub
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LukBed @lightbend-cla-validator @ennru