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

feat(cli): early protection for invalid prometheus prefix #1107

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

jansegre
Copy link
Member

@jansegre jansegre commented Aug 7, 2024

Motivation

We had a report from someone accidentally using an invalid prefix through HATHOR_PROMETHEUS_PREFIX which had a message that was hard to understand.

Acceptance Criteria

  • Check for invalid Prometheus prefix when building and give a more helpful message.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@jansegre jansegre requested review from msbrogli and glevco August 7, 2024 14:01
@jansegre jansegre self-assigned this Aug 7, 2024
@jansegre jansegre force-pushed the feat/invalid-prometheus-prefix-protection branch from f32d523 to d18ce78 Compare August 7, 2024 14:02
Copy link

github-actions bot commented Aug 7, 2024

🐰Bencher

ReportThu, August 15, 2024 at 13:06:32 UTC
Projecthathor-core
Branchfeat/invalid-prometheus-prefix-protection
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
Latency Lower Boundary
nanoseconds (ns) | (%)
Latency Upper Boundary
nanoseconds (ns) | (%)
sync-v2 (up to 20000 blocks)✅ (view plot)104,169,901,037.00 (-9.03%)91,611,202,847.68 (87.94%)137,416,804,271.52 (75.81%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.86%. Comparing base (445be1c) to head (79c03e9).
Report is 2 commits behind head on master.

Files Patch % Lines
hathor/builder/resources_builder.py 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1107      +/-   ##
==========================================
- Coverage   84.87%   84.86%   -0.01%     
==========================================
  Files         315      315              
  Lines       24059    24070      +11     
  Branches     3644     3647       +3     
==========================================
+ Hits        20419    20428       +9     
- Misses       2927     2929       +2     
  Partials      713      713              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

msbrogli
msbrogli previously approved these changes Aug 9, 2024
hathor/builder/resources_builder.py Outdated Show resolved Hide resolved
@jansegre jansegre force-pushed the feat/invalid-prometheus-prefix-protection branch from d18ce78 to 0f99e75 Compare August 14, 2024 15:48
@jansegre jansegre force-pushed the feat/invalid-prometheus-prefix-protection branch from 0f99e75 to e9b57d6 Compare August 14, 2024 16:34
@jansegre jansegre force-pushed the feat/invalid-prometheus-prefix-protection branch from e9b57d6 to 79c03e9 Compare August 15, 2024 13:05
@jansegre jansegre merged commit 3681579 into master Aug 15, 2024
12 checks passed
@jansegre jansegre deleted the feat/invalid-prometheus-prefix-protection branch August 15, 2024 13:06
@jansegre jansegre mentioned this pull request Oct 4, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants