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.

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

Better URL comparison for metrics #206

Merged
merged 10 commits into from
Sep 19, 2019
Merged

Better URL comparison for metrics #206

merged 10 commits into from
Sep 19, 2019

Conversation

nwmac
Copy link
Member

@nwmac nwmac commented Sep 9, 2019

When comparing URLs to match a metrics endpoint with other endpoints that it might provide metrics for, we now use a better comparison that doesn't just compare strings - which fixes issues when the mismatch is only in the port number due to default port.

@nwmac nwmac self-assigned this Sep 9, 2019
@nwmac nwmac added the ready for review Ready for review label Sep 9, 2019
@codecov-io
Copy link

codecov-io commented Sep 9, 2019

Codecov Report

Merging #206 into v2-master will decrease coverage by <.01%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           v2-master     #206      +/-   ##
=============================================
- Coverage      53.18%   53.18%   -0.01%     
=============================================
  Files            918      918              
  Lines          25290    25290              
  Branches        4336     4336              
=============================================
- Hits           13451    13450       -1     
- Misses         11839    11840       +1

Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

Haven't got an environment to test on at the moment but code looks good

@nwmac nwmac merged commit ca4162e into v2-master Sep 19, 2019
@nwmac nwmac deleted the better-url-comparison branch September 19, 2019 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants