Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Don't support ffwd over http anymore, use semantic-metrics 1.1.8 where the lib is not included. #342

Merged
merged 8 commits into from
May 6, 2021

Conversation

klaraward
Copy link
Contributor

@klaraward klaraward commented Apr 21, 2021

Can not find any usages of the config ffwd.type: http in the Spotify code base, and I believe @lmuhlha said she wants to get rid of the ffwd-http-client library - #326 (comment)

Include the old version of ffwd-http-client lib to make sure downstream libraries get that as a transitive dependency and can use the shaded packages.

klaraward added a commit that referenced this pull request Apr 22, 2021
Reverts #330, which causes problems in downstream dependencies because of the removal of all shaded libraries in ffwd-http-client
Will update to new version of semantic-metrics once #342 has been merged
@klaraward
Copy link
Contributor Author

We still have a problem with using a new version of semantic-metrics, because it brings in the new ffwd-http-client without the shaded dependencies, that too many people depend on.

@klaraward klaraward changed the title Don't support ffwd over http anymore Don't support ffwd over http anymore, use semantic-metrics 1.1.8 where the lib is not included. Apr 27, 2021
Copy link
Member

@lepistone lepistone left a comment

Choose a reason for hiding this comment

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

Sounds great together with spotify/semantic-metrics#108. Thank you!

@klaraward klaraward marked this pull request as ready for review May 6, 2021 07:14
@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #342 (bfd92c6) into 1.x (dd500d1) will increase coverage by 0.07%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                1.x     #342      +/-   ##
============================================
+ Coverage     71.96%   72.04%   +0.07%     
+ Complexity      676      673       -3     
============================================
  Files           138      137       -1     
  Lines          2904     2862      -42     
  Branches        169      167       -2     
============================================
- Hits           2090     2062      -28     
+ Misses          776      763      -13     
+ Partials         38       37       -1     
Impacted Files Coverage Δ Complexity Δ
...in/java/com/spotify/apollo/metrics/FfwdConfig.java 54.83% <0.00%> (+4.83%) 2.00 <0.00> (-1.00) ⬆️
...main/java/com/spotify/apollo/core/ServiceImpl.java 90.37% <0.00%> (-1.04%) 35.00% <0.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 dd500d1...bfd92c6. Read the comment docs.

@@ -162,19 +162,20 @@
</dependency>
<dependency>
<groupId>com.spotify.metrics</groupId>
<artifactId>semantic-metrics-core</artifactId>
<artifactId>semantic-metrics-bom</artifactId>
<version>${semantic-metrics.version}</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

If we will use the semantic-metrics-bom now, is this parameter required?

@klaraward klaraward merged commit 69b4967 into 1.x May 6, 2021
@klaraward klaraward deleted the no-ffwd-http-client branch May 6, 2021 12:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants