achave11-ucsc
commented
7 months ago This patch was produced during PL:
Subject: [PATCH] [r] Index dcp35 in prod (#5949)
---
Index: src/azul/service/async_manifest_service.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/azul/service/async_manifest_service.py b/src/azul/service/async_manifest_service.py
--- a/src/azul/service/async_manifest_service.py (revision 9c69f828897a5b9b1c37f96dff7aaf94b04aca27)
+++ b/src/azul/service/async_manifest_service.py (date 1707945397482)
@@ -125,7 +125,7 @@
token = Token.first(execution_id)
input = json.dumps(input)
try:
- # If there already is an exception of the given name, and if that
+ # If there already is an execution of the given name, and if that
# execution is still ongoing and was given the same input as what we
# pass here, `start_execution` will succeed idempotently
execution = self._sfn.start_execution(stateMachineArn=self.machine_arn,
@@ -133,12 +133,12 @@
input=input)
except self._sfn.exceptions.ExecutionAlreadyExists:
# This exception indicates that there is already an execution with
- # the given name but that it is has ended, or that its input differs
+ # the given name but that it has ended, or that its input differs
# from what we were passing now. The latter case is unexpected and
# therefore constitues an error. In the former case we return the
# token so that the client has to make another request to actually
# obtain the resulting manifest. Strictly speaking, we could return
- # the manifest here but it keeps the control flow simpler. This is
+ # the manifest here, but it keeps the control flow simpler. This is
# benevolent race is rare enough to not worry about optimizing.
execution = self._sfn.describe_execution(executionArn=execution_arn)
if execution['input'] != input:
Index: src/azul/service/manifest_service.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/azul/service/manifest_service.py b/src/azul/service/manifest_service.py
--- a/src/azul/service/manifest_service.py (revision 9c69f828897a5b9b1c37f96dff7aaf94b04aca27)
+++ b/src/azul/service/manifest_service.py (date 1707945678727)
@@ -743,8 +743,15 @@
try:
encoded_file_name = tagging[self.file_name_tag]
except KeyError:
- # Can't be absent under S3's strong consistency
- assert False, (object_key, self.file_name_tag)
+ # Some manifest generators apply the tag after creating
+ # the object. If we happened to be right inbetween the
+ # creation and application of tags, we pretend that the
+ # manifest object doesn't exist, assuming that the caller
+ # will tell the client to check back later. Note that this
+ # might even involve the attempted creation of a step-function
+ # execution but because step-functions are idempotent the result
+ # of the … <TODO>
+ return None
else:
encoded_file_name = encoded_file_name.encode('ascii')
return base64.urlsafe_b64decode(encoded_file_name).decode('utf-8')
hannes-ucsc
dsotirho-ucsc
The following logs exhibit the race when a service request for a manifest, with an ongoing generation, fails due to the manifest missing the name tag, which are then applied after the service request had return a 500 status code.
CloudWatch Logs Insights
region: us-east-1
log-group-names: /aws/lambda/azul-service-prod, /aws/lambda/azul-service-prod-manifest
start-time: -172800s
end-time: 0s
query-string:
~~~^^^^^^^^^^^^^^^^^^^^ KeyError: 'azul_file_name' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/var/task/chalice/app.py", line 1917, in _get_view_function_response response = view_function(**function_args) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/app.py", line 1557, in fetch_file_manifest_get return _file_manifest(fetch=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/app.py", line 1583, in _file_manifest return app.manifest_controller.get_manifest_async(query_params=query_params, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/azul/service/manifest_controller.py", line 139, in get_manifest_async manifest = self.service.get_cached_manifest(format=format, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/azul/service/manifest_service.py", line 651, in get_cached_manifest return self._get_cached_manifest(generator_cls, manifest_key) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/azul/service/manifest_service.py", line 690, in _get_cached_manifest file_name = self._get_cached_manifest_file_name(generator_cls, manifest_key) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/azul/service/manifest_service.py", line 747, in _get_cached_manifest_file_name assert False, (object_key, self.file_name_tag) AssertionError: ('manifests/52f339c2-0874-58b0-82a3-eef96ee01af5.58e28026-9ea8-5083-928d-0517707b35a2.tsv', 'azul_file_name')~~~^^^^^^^^^^^^^^^^^^^^ KeyError: 'azul_file_name' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/var/task/chalice/app.py", line 1917, in _get_view_function_response response = view_function(**function_args) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/app.py", line 1557, in fetch_file_manifest_get return _file_manifest(fetch=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/app.py", line 1583, in _file_manifest return app.manifest_controller.get_manifest_async(query_params=query_params, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/var/task/azul/service/manifest_controller.py", line 139, in get_manifest_async manifest = self.service.get_cached_m