Skip to content

Commit

Permalink
Merge pull request #236 from CMSTrackerDPG/develop [closes #232]
Browse files Browse the repository at this point in the history
  • Loading branch information
@nothingface0
nothingface0 authored Jul 7, 2023
2 parents 8cf600e + 47abeae commit 07a9017
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 7 deletions.
43 changes: 39 additions & 4 deletions certifier/templates/certifier/certify.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,19 @@ <h1>
<div class="col-md-4">
<div class="card shadow-sm bg-white">
<div class="card-body">
<h6 class="card-title text-primary">Type</h6>
<div class="row">
<div class="col-11">
<h6 class="card-title text-primary">Type</h6>
</div>
<div class="col-1">
{% comment %} Allow editing of OMS attributes {% endcomment %}
{% if form.external_info_complete.value %}
<div class="row justify-content-center">
<a class="btn btn-light" title="Edit OMS info" href="./?external_info_complete=false"><i class="bi bi-pencil-fill pl-1"></i></a>
</div>
{% endif %}
</div>
</div>
<div class="row m-1">
<div class="col-md-4 text-right">
<strong>Run type</strong>
Expand Down Expand Up @@ -155,8 +167,19 @@ <h6 class="card-title text-primary">Type</h6>
<div class="col-md-4">
<div class="card shadow-sm">
<div class="card-body">

<h6 class="card-title text-success">Lumi</h6>
<div class="row">
<div class="col-11">
<h6 class="card-title text-success">Lumi</h6>
</div>
<div class="col-1">
{% comment %} Allow editing of OMS attributes {% endcomment %}
{% if form.external_info_complete.value %}
<div class="row justify-content-center">
<a class="btn btn-light" title="Edit OMS info" href="./?external_info_complete=false"><i class="bi bi-pencil-fill pl-1"></i></a>
</div>
{% endif %}
</div>
</div>
<div class="row m-1">
<div class="col-md-5 text-right">
<strong>Lumisections</strong>
Expand Down Expand Up @@ -237,7 +260,19 @@ <h6 class="card-title text-success">Lumi</h6>
<div class="col-md-4">
<div class="card shadow-sm">
<div class="card-body">
<h6 class="card-title text-info">Fill</h6>
<div class="row">
<div class="col-11">
<h6 class="card-title text-info">Fill</h6>
</div>
<div class="col-1">
{% comment %} Allow editing of OMS attributes {% endcomment %}
{% if form.external_info_complete.value %}
<div class="row justify-content-center">
<a class="btn btn-light" title="Edit OMS info" href="./?external_info_complete=false"><i class="bi bi-pencil-fill pl-1"></i></a>
</div>
{% endif %}
</div>
</div>
<div class="row m-1">
<div class="col-md-5 text-right">
<strong>Fill number</strong>
Expand Down
6 changes: 6 additions & 0 deletions certifier/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from oms.exceptions import (
OmsApiFillNumberNotFound,
OmsApiRunNumberNotFound,
OmsApiDataInvalidError,
RunRegistryNoAvailableDatasets,
RunRegistryReconstructionNotFound,
)
Expand Down Expand Up @@ -204,6 +205,7 @@ def dispatch(self, request, *args, **kwargs):
ParseError,
OmsApiRunNumberNotFound,
OmsApiFillNumberNotFound,
OmsApiDataInvalidError,
) as e:
# If OMS API does not contain the info required,
# or OMS is unreachable, create the run with minimal
Expand All @@ -222,6 +224,10 @@ def get(self, request, run_number: int, reco: str = None):
logger.debug(
f"Requesting certification of run {run_number} {reco if reco else ''}"
)
# Allow GUI user to force update of the OMS data
# A bit of a machete code, in order to
if request.GET.get("external_info_complete", None) == "false":
self.external_info_complete = False

context = {
"run_number": run_number,
Expand Down
2 changes: 1 addition & 1 deletion dqmhelper/settings.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
messages.ERROR: "danger",
}
# Version to display in order to keep track of changes
CERTHELPER_VERSION = "1.10.3"
CERTHELPER_VERSION = "1.11.0"

# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
BASE_DIR = Path(__file__).resolve().parent.parent
Expand Down
7 changes: 7 additions & 0 deletions oms/exceptions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ class OmsApiRunNumberNotFound(Exception):
"""


class OmsApiDataInvalidError(Exception):
"""
Raised when data from OMS violates a constraint in our DB,
e.g. Colliding bunches < 0
"""


class RunRegistryNoAvailableDatasets(Exception):
"""
Raised when no datasets were found in RunRegistry
Expand Down
20 changes: 18 additions & 2 deletions oms/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from oms.exceptions import (
OmsApiFillNumberNotFound,
OmsApiRunNumberNotFound,
OmsApiDataInvalidError,
RunRegistryNoAvailableDatasets,
RunRegistryReconstructionNotFound,
)
Expand Down Expand Up @@ -161,7 +162,20 @@ def oms_retrieve_fill(fill_number: int) -> OmsFill: # pragma: no cover
try:
with transaction.atomic():
OmsFill.objects.create(**fill_kwargs)
except IntegrityError:
except IntegrityError as e:
logger.error(f"Failed to create OmsFill object in DB: {repr(e)}")
if "violates check constraint" in repr(e):
raise OmsApiDataInvalidError(repr(e))
# This try-except block was probably added for the case where more than one users simultaneously
# ask to certify the same run, meaning that only one call of this function
# will actually manage to create the entry, all others throwing an Integrity Error.
# Not raising an exception here, though, allows ALL kinds of IntegrityErrors to pass through,
# including ones that have to do with the actual data from OMS being bad.
# An example was an incident where the bunches_colliding that OMS was returning was negative,
# raising an IntegrityError here, which failed to create the OmsFill entry, hence
# raising an exception on the return statement without a descriptive message.
# For this reason, it's better to just crash early, with a meaningful reason.
# There's, of course, room for improvement here.
OmsFill.objects.filter(fill_number=fill_number).update(**fill_kwargs)

return OmsFill.objects.get(fill_number=fill_number)
Expand Down Expand Up @@ -250,10 +264,12 @@ def oms_retrieve_run(run_number: int) -> OmsRun: # pragma: no cover
run_kwargs["lumisections"] = get_oms_lumisection_count(run_number)

try:
# Make sure that, if more than one user requests a run that had not been added to the DB,
# it will only be added once.
with transaction.atomic():
OmsRun.objects.create(fill=fill, **run_kwargs)
except IntegrityError as e:
logger.warning(f"{e} trying to create OmsRun")
logger.error(f"Failed to create OmsRun object: {repr(e)}")
OmsRun.objects.filter(run_number=run_number).update(**run_kwargs)

return OmsRun.objects.get(run_number=run_number)

0 comments on commit 07a9017

Please sign in to comment.