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

handle ClientError raised after key is missing during DyanmoDB table.get_item #42408

Merged

Conversation

Lee-W
Copy link
Member

@Lee-W Lee-W commented Sep 23, 2024

Why

Since moto 5.0.15, it validates whether the key exists which might indicate we does not check whether the key exist in our sensor.

What

Add an additional check to handle boto ClientError


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:amazon-aws AWS/Amazon - related issues labels Sep 23, 2024
@Lee-W Lee-W marked this pull request as draft September 23, 2024 13:33
@eladkal eladkal added upgrade to newer dependencies If set, upgrade to newer dependencies is forced canary When set on PR running from apache repo - behave as canary run labels Sep 23, 2024
@eladkal eladkal closed this Sep 23, 2024
@eladkal eladkal reopened this Sep 23, 2024
@eladkal
Copy link
Contributor

eladkal commented Sep 23, 2024

We are having some canary build failure around dynamo db:

FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBValueSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema
4441
FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBMultipleValuesSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema

https://github.com/apache/airflow/actions/runs/10989610037/job/30508381216#step:7:4458

Looks like botocore changed the dynamodb endpoints boto/botocore#3263 (comment)

Added canary and upgrade labels to rerun the CI with the updated versions

Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

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

Change looks reasonable.

@vincbeck
Copy link
Contributor

vincbeck commented Sep 23, 2024

Could you add/update unit test to cover this new branch?

@eladkal
Copy link
Contributor

eladkal commented Sep 23, 2024

We are having some canary build failure around dynamo db:

FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBValueSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema
4441
FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBMultipleValuesSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema

https://github.com/apache/airflow/actions/runs/10989610037/job/30508381216#step:7:4458

Looks like botocore changed the dynamodb endpoints boto/botocore#3263 (comment)

Added canary and upgrade labels to rerun the CI with the updated versions

Fixed in #42419

@Lee-W Lee-W force-pushed the import-DynamoDBValueSensor-key-error-checking branch from 85ded41 to 06e3dfd Compare September 24, 2024 08:48
@Lee-W Lee-W marked this pull request as ready for review September 24, 2024 10:18
@Lee-W
Copy link
Member Author

Lee-W commented Sep 24, 2024

Could you add/update unit test to cover this new branch?

Sure! just added

@Lee-W
Copy link
Member Author

Lee-W commented Sep 24, 2024

We are having some canary build failure around dynamo db:

FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBValueSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema
4441
FAILED tests/providers/amazon/aws/sensors/test_dynamodb.py::TestDynamoDBMultipleValuesSensor::test_sensor_with_pk - botocore.exceptions.ClientError: An error occurred (ValidationException) when calling the GetItem operation: The provided key element does not match the schema

https://github.com/apache/airflow/actions/runs/10989610037/job/30508381216#step:7:4458
Looks like botocore changed the dynamodb endpoints boto/botocore#3263 (comment)
Added canary and upgrade labels to rerun the CI with the updated versions

Fixed in #42419

Thanks! This one was originally meant to solve that, but seems to be the wrong fix 🤦‍♂️

@vincbeck vincbeck merged commit b9629d9 into apache:main Sep 24, 2024
61 checks passed
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers canary When set on PR running from apache repo - behave as canary run provider:amazon-aws AWS/Amazon - related issues upgrade to newer dependencies If set, upgrade to newer dependencies is forced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants