search

spotify / async-google-pubsub-client

[SUNSET] Async Google Pubsub Client
Apache License 2.0
157 stars 40 forks source link

Add minimum support for labels #42

Closed RickardZwahlen closed 6 years ago

RickardZwahlen commented 6 years ago

Pubsub has added labels in the latest alpha. The current version of this client crashes with a jackson UnrecognizedPropertyException in the TopicBuilder if it fetches a topic that has labels added to it. The same applies to subscriptions.

This change adds the minimum support for avoiding those crashes. It does not add support for managing labels through the client.

codecov-io commented 6 years ago

Codecov Report

Merging #42 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master     #42   +/-   ##
========================================
  Coverage      76.1%   76.1%           
  Complexity      191     191           
========================================
  Files            26      26           
  Lines           904     904           
  Branches         59      59           
========================================
  Hits            688     688           
  Misses          183     183           
  Partials         33      33
Impacted Files Coverage Δ Complexity Δ
.../com/spotify/google/cloud/pubsub/client/Topic.java 77.77% <ø> (ø) 6 <0> (ø) :arrow_down:
...otify/google/cloud/pubsub/client/Subscription.java 81.81% <ø> (ø) 6 <0> (ø) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 25e51d3...e61cf17. Read the comment docs.

danielnorberg commented 6 years ago

Perhaps the mapper should be configured to ignore unknown fields when deserializing in order to be forwards compatible.

RickardZwahlen commented 6 years ago

It seems like it was added already. We were just a couple of versions behind.

It still makes sense to add support for labels at some point, but it is probably better to do it properly, since the current client won't break if it receives a payload with unknown fields.

danielnorberg commented 6 years ago

👍 As this library is currently sunsetting and on life-support, would it be possible / make sense for you to switch to the official library?

RickardZwahlen commented 6 years ago

That is the plan, when we find the time. For now the latest version works, so I'll close this PR. Thanks!