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

Improve server logic #235

Merged
merged 10 commits into from
Nov 21, 2024
Prev Previous commit
Next Next commit
Log information mismatch in case of not certified response
nadzyah committed Nov 21, 2024

Verified

This commit was signed with the committer’s verified signature.
Ant0wan Antoine Barthelemy
commit 5a3c7d34168d08d865ceddbd8955fb3cca3fa754
17 changes: 16 additions & 1 deletion server/hwapi/endpoints/certification/certification.py
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@
# Nadzeya Hutsko <[email protected]>
"""The endpoints for working with certification status"""


import logging
from fastapi import APIRouter, Depends
from sqlalchemy.orm import Session

@@ -61,6 +61,7 @@ def check_certification(
vendor = repository.get_vendor_by_name(db, system_info.vendor)
if not vendor:
return NotCertifiedResponse()

# Match against board and bios
try:
board, bios_list = logic.find_main_hardware_components(
@@ -70,16 +71,30 @@ def check_certification(
db, system_info.architecture, board, bios_list
)
except ValueError:
logging.error(
(
"Hardware cannot be found. Machine vendor: %s, model: %s"
", board model: %s, board version: %s, bios version: %s"
),
system_info.vendor,
system_info.model,
system_info.board.product_name,
system_info.board.version,
system_info.bios.version if system_info.bios else None,
)
Comment on lines +73 to +83

Choose a reason for hiding this comment

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

This endpoint checks whether a hardware is certified or not. If it's not certified then it responds with NotCertifiedResponse. If so then it is expected behavior to find not certified device where the board doesn't match for instance. Hence it's not an error. So perhaps it's not correct to log an error here. Additionally, this is the API any logs made here are not shown to the client so we shouldn't be logging anything apart from real errors, warnings or perhaps debug info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Matias specifically asked for logging hardware mismatch data since we'll probably have to adjust the logic according to, for instance, how many certified machines run a newer bios version, different from which we used for issuing a certificate

Choose a reason for hiding this comment

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

This sounds like we need to store this data to be able to query it rather than just log it. I'm not sure if simply logging will help us get such information. @mz2 wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair point, it probably is better that way. I was imagining deriving metrics from logs, but explicitly storing the full data is probably a better idea.

Copy link
Collaborator Author

@nadzyah nadzyah Nov 21, 2024

Choose a reason for hiding this comment

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

After discussing it with @omar-selo, we've decided to add this change in another PR and create a table for storing hardware mismatches
Here is the Jira ticket for it: https://warthogs.atlassian.net/browse/C3-945

return NotCertifiedResponse()

bios = repository.get_machine_bios(db, related_machine.id)
related_releases, kernels = repository.get_releases_and_kernels_for_machine(
db, related_machine.id
)

# Match against CPU codename
if not logic.check_cpu_compatibility(db, related_machine, system_info.processor):
return response_builders.build_related_certified_response(
db, related_machine, board, bios, related_releases, kernels
)

# Check OS release
release_from_request = repository.get_release_object(
db, system_info.os.version, system_info.os.codename