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

fix(consumption-profile): use family and TDP parameters in CPU completion #377

Merged
merged 1 commit into from
Mar 1, 2025

Conversation

Shillaker
Copy link
Contributor

@Shillaker Shillaker commented Feb 24, 2025

Problems

The CPU consumption profile endpoint:

  • Does not currently use the family and TDP parameters from the name completion
  • Returns a 500 error if the CPU name fuzzy-match fails

Fix

  • Use all relevant values from fuzzy matching to compute consumption profile
  • Tolerate unrecognised CPU name (by taking the default)

@Shillaker Shillaker changed the title fix(consumption-profile): use family and tdp parameters in CPU completion fix(consumption-profile): use family and TDP parameters in CPU completion Feb 24, 2025
@Shillaker Shillaker force-pushed the fix/tdp-consumption-profile branch from 013d441 to 1033344 Compare February 25, 2025 15:39
@Shillaker Shillaker force-pushed the fix/tdp-consumption-profile branch from 1033344 to 4f2297b Compare February 25, 2025 15:40
@Shillaker Shillaker marked this pull request as ready for review February 25, 2025 15:41
@da-ekchajzer
Copy link
Collaborator

Hum, that is weird, I will have a look this weekend. Is this only a problem when using the CPU route ? When assessing a server or a cloud instance, the CPU TDP is used ? Do you have an exemple that doesn't work ?

@Shillaker
Copy link
Contributor Author

Shillaker commented Feb 26, 2025

Yeah it's just the implementation of this endpoint. AFAICT consumption profiles + name lookup works fine elsewhere.

You can test it in Swagger (here). If you send a request with just a CPU name, and vary the name, the results don't change (same if you add a TDP). If you also set some junk name like foobar you get a 500.

@da-ekchajzer
Copy link
Collaborator

Perfect tested works for me ! Thank you for the bug fix.

@da-ekchajzer da-ekchajzer merged commit 8b0c814 into Boavizta:main Mar 1, 2025
3 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