Skip to content

Conversation

@vprashar2929
Copy link
Contributor

@vprashar2929 vprashar2929 commented Oct 22, 2024

This commit updates the condition to check the length of PROM_THIRDPARTY_METRICS instead of comparing it to [""]

Checklist for PR Author


In addition to approval, the author must confirm the following check list:

  • Run the following command to format your code:

    make fmt
  • Create issues for unresolved comments and link them to this PR. Use one of the following labels:

    • must-fix: The logic appears incorrect and must be addressed.
    • minor: Typos, minor issues, or potential refactoring for better readability.
    • nit: Trivial issues like extra spaces, commas, etc.

@rootfs rootfs requested a review from sthaha October 22, 2024 12:33
Copy link
Contributor

@sthaha sthaha left a comment

Choose a reason for hiding this comment

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

@vprashar2929 how about we fix the check (if PROM_.. ) instead of adding dummy ""

This commit updates the condition to check the length
of `PROM_THIRDPARTY_METRICS` instead of comparing it to `[""]`

Signed-off-by: Vibhu Prashar <vibhu.sharma2929@gmail.com>
@vprashar2929 vprashar2929 changed the title fix(prom): set default value of PROM_THIRDPARTY as [""] fix: check length of PROM_THIRDPARTY_METRICS Oct 22, 2024
@vprashar2929 vprashar2929 requested a review from sthaha October 22, 2024 16:05
Comment on lines +151 to 154
elif len(PROM_THIRDPARTY_METRICS) > 0:
queries = [m for m in available_metrics if args.metric_prefix in m or m in PROM_THIRDPARTY_METRICS]
else:
queries = [m for m in available_metrics if args.metric_prefix in m]
Copy link
Contributor

Choose a reason for hiding this comment

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

is this condition needed any more since PROM_THIRD.. is a list already ?

@sthaha sthaha merged commit fc86bf2 into sustainable-computing-io:main Oct 23, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants