spring-attic / spring-cloud-gcp

Integration for Google Cloud Platform APIs with Spring

Apache License 2.0

705 stars 693 forks source link

Remove exclusion of protobuf-java from MySQL driver #2457

Closed mkatircioglu closed 4 years ago

mkatircioglu commented 4 years ago

Closes #gh-957

codecov[bot] commented 4 years ago

Codecov Report

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

@@            Coverage Diff            @@
##             master    #2457   +/-   ##
=========================================
  Coverage     74.05%   74.05%           
  Complexity     2119     2119           
=========================================
  Files           264      264           
  Lines          7663     7663           
  Branches        790      790           
=========================================
  Hits           5675     5675           
  Misses         1620     1620           
  Partials        368      368

Flag	Coverage Δ	Complexity Δ
#integration	`?`	`?`
#unittests	`74.05% <ø> (ø)`	`2119.00 <ø> (ø)`

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 8768cbc...b57fda7. Read the comment docs.

mkatircioglu commented 4 years ago

Hi @meltsufin. I'm not totally sure how I can verify the issue therefore I have created PR as draft. After seeing there is no error on CI pipeline, I have also tested with spring-cloud-gcp-sql-mysql-sample whether it still works or not. Is it enough for you or would you like me to do something else? I would be very happy if you can guide me.

meltsufin commented 4 years ago

@mkatircioglu This testing is sufficient. However, checking mysql-connector-java pom.xml, I see that it still includes protobuf-java as a non-optional dependency. The version of the dependency is 3.6.1 and is probably no longer incompatible to cause problems, but it can cause problems in the future, potentially. @saturnism What do you think? I don't have access to the original bug on mysql.com. Can you see what the outcome was there? https://bugs.mysql.com/bug.php?id=91999

saturnism commented 4 years ago

it seems like i lost access too :( there were some back and forth on the driver side, so atm, i think it's ok to be defensive and exclude for the moment. will need to see the driver version in the bom and whether they are still using the correct scope.