Skip to content

Commit

Permalink
[R] Fix: Manifest request fails if filter params are rearranged (#6417,…
Browse files Browse the repository at this point in the history
… PR #6443)
  • Loading branch information
dsotirho-ucsc committed Aug 26, 2024
2 parents be8a0bc + 737bde7 commit 751e60c
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 82 deletions.
Binary file not shown.
27 changes: 17 additions & 10 deletions common.mk
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,27 @@ check_python: check_venv
echo -e "\nPATH lookup yields a 'pip' executable from outside the virtualenv\n"; \
false; \
fi
@if ! python -c "import sys; sys.exit(0 if '.'.join(map(str, sys.version_info[:3])) == '${azul_python_version}' else 1)"; then \
echo -e "\nLooks like Python ${azul_python_version} is not installed or active in the current virtualenv\n"; \
@if ! python -c "import sys, os; \
p = lambda v: tuple(map(int, v.split('.'))); \
v = os.environ['azul_python_version']; \
sys.exit(0 if sys.version_info[:3] == p(v) else 1)"; then \
echo -e "\nLooks like Python ${azul_python_version} is not installed\n"; \
false; \
fi
@if ! python -c "import sys; exec('try: import chalice\nexcept: sys.exit(1)\nelse: sys.exit(0)')"; then \
@if ! python -c "import sys; \
exec('try: import chalice\nexcept: sys.exit(1)\nelse: sys.exit(0)')"; then \
echo -e "\nLooks like some requirements are missing. Please run 'make requirements'\n"; \
false; \
fi
@if ! python -c "import sys, wheel as w; \
from pkg_resources import parse_version as p; \
sys.exit(0 if p(w.__version__) >= p('0.32.3') else 1)"; then \
p = lambda v: tuple(map(int, v.split('.'))); \
sys.exit(0 if p(w.__version__) >= p('0.32.3') else 1)"; then \
echo -e "\nLooks like the `wheel` package is outdated or missing. See README for instructions on how to fix this.\n"; \
false; \
fi
@if ! python -c "import sys; \
from chalice import chalice_version as v; \
from pkg_resources import parse_version as p; \
p = lambda v: tuple(map(int, v.split('.'))); \
sys.exit(0 if p(v) == p('1.30.0') else 1)"; then \
echo -e "\nLooks like chalice is out of date. Please run 'make requirements'\n"; \
false; \
Expand Down Expand Up @@ -74,10 +78,13 @@ check_docker:
.PHONY: check_aws
check_aws: check_python
@if ! python -c "import os, sys, boto3 as b; \
sys.exit(0 if os.environ.get('TRAVIS') == 'true' or \
b.client('sts').get_caller_identity()['Account'] == os.environ['AZUL_AWS_ACCOUNT_ID'] else 1)"; then \
echo -e "\nLooks like there is a mismatch between AZUL_AWS_ACCOUNT_ID and the currently active AWS credentials. \
\nCheck the output from 'aws sts get-caller-identity' against the value of that environment variable.\n"; \
expected = os.environ['AZUL_AWS_ACCOUNT_ID']; \
actual = b.client('sts').get_caller_identity()['Account']; \
sys.exit(0 if actual == expected else 1)"; then \
echo Looks like there is a mismatch between AZUL_AWS_ACCOUNT_ID \
and the currently active AWS credentials. ; \
echo Check the output from \'aws sts get-caller-identity\' against the \
value of that environment variable. ; \
false; \
fi

Expand Down
8 changes: 4 additions & 4 deletions requirements.all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ blinker==1.8.2
boto3==1.28.63
boto3-stubs==1.28.63
botocore==1.31.63
botocore-stubs==1.35.5
botocore-stubs==1.35.6
brotli==1.1.0
cachetools==5.5.0
certifi==2024.7.4
Expand Down Expand Up @@ -49,7 +49,7 @@ google-cloud-core==2.4.1
google-cloud-storage==2.12.0
google-crc32c==1.5.0
google-resumable-media==2.7.2
googleapis-common-protos==1.63.2
googleapis-common-protos==1.64.0
greenlet==3.0.3
grpcio==1.66.0
grpcio-status==1.62.3
Expand Down Expand Up @@ -103,7 +103,7 @@ pygithub==1.56
pyjwt==2.9.0
pynacl==1.5.0
pyopenssl==24.2.1
pyparsing==3.1.2
pyparsing==3.1.4
pyrsistent==0.20.0
python-dateutil==2.9.0.post0
python-dxf==11.4.0
Expand Down Expand Up @@ -142,4 +142,4 @@ wrapt==1.16.0
www-authenticate==0.9.2
xmltodict==0.13.0
zope.event==5.0
zope.interface==7.0.1
zope.interface==7.0.2
6 changes: 3 additions & 3 deletions requirements.dev.trans.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
blessed==1.20.0
blinker==1.8.2
botocore-stubs==1.35.5
botocore-stubs==1.35.6
brotli==1.1.0
click==8.1.7
colorama==0.4.4
Expand Down Expand Up @@ -41,7 +41,7 @@ pycodestyle==2.9.1
pyflakes==2.5.0
pyjwt==2.9.0
pynacl==1.5.0
pyparsing==3.1.2
pyparsing==3.1.4
pyrsistent==0.20.0
python-editor==1.0.4
pyzmq==26.2.0
Expand All @@ -59,4 +59,4 @@ wcwidth==0.2.13
www-authenticate==0.9.2
xmltodict==0.13.0
zope.event==5.0
zope.interface==7.0.1
zope.interface==7.0.2
2 changes: 1 addition & 1 deletion requirements.trans.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ cryptography==43.0.0
google-cloud-core==2.4.1
google-crc32c==1.5.0
google-resumable-media==2.7.2
googleapis-common-protos==1.63.2
googleapis-common-protos==1.64.0
grpcio==1.66.0
grpcio-status==1.62.3
http_sfv==0.9.9
Expand Down
31 changes: 18 additions & 13 deletions src/azul/service/async_manifest_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,37 @@ def start_generation(self, execution_id: bytes, input: JSON) -> Token:
execution_arn = self.execution_arn(execution_name)
# The input contains the verbatim manifest key as JSON while the ARN
# contains the encoded hash of the key so this log line is useful for
# associating the hash with the key for diagnostic purposes
# associating the hash with the key for diagnostic purposes.
log.info('Starting execution %r for input %r', execution_arn, input)
token = Token.first(execution_id)
input = json.dumps(input)
try:
# 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
# pass here, `start_execution` will succeed idempotently.
execution = self._sfn.start_execution(stateMachineArn=self.machine_arn,
name=execution_name,
input=input)
input=json.dumps(input))
except self._sfn.exceptions.ExecutionAlreadyExists:
# This exception indicates that there is already an execution with
# 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
# benevolent race is rare enough to not worry about optimizing.
# from what we were passing just now. The latter case is unexpected
# because any part of the input that affects the output is covered
# in the manifest hash and therefore the execution name. Any part of
# the input not affecting the output is constant and can only change
# with the source code which would have resulted in a different
# execution name.
#
# 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 benevolent race is not probable
# enough to warrant an optimization.
execution = self._sfn.describe_execution(executionArn=execution_arn)
if execution['input'] != input:
raise InvalidGeneration(token)
else:
if input == json.loads(execution['input']):
log.info('A completed execution %r already exists', execution_arn)
return token
else:
raise InvalidGeneration(token)
else:
assert execution_arn == execution['executionArn'], execution
log.info('Started execution %r or it was already running', execution_arn)
Expand Down
4 changes: 2 additions & 2 deletions src/azul/service/manifest_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,11 @@ def get_manifest_async(self,
}
# Manifest keys for catalogs with long names would be too
# long to be used directly as state machine execution names.
execution_key = manifest_key.hash
execution_id = manifest_key.hash
# ManifestGenerationState is also JSON but there is no way
# to express that since TypedDict rejects a co-parent class.
input: JSON = cast(JSON, state)
token = self.async_service.start_generation(execution_key, input)
token = self.async_service.start_generation(execution_id, input)
else:
manifest_key = manifest.manifest_key
else:
Expand Down
Loading

0 comments on commit 751e60c

Please sign in to comment.