You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "amoghrajesh (via GitHub)" <gi...@apache.org> on 2026/05/06 13:21:21 UTC
[PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
amoghrajesh opened a new pull request, #66473:
URL: https://github.com/apache/airflow/pull/66473
moto changed the HTTP status code returned for DeleteModel on a non-existent model from 404 to 400 across versions. Accept both codes so the test passes against Airflow 3.2.1 compat environments.
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the documentation.
Feel free to ping (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
Fix for this error:
```
___________________________________________________________________________ TestSageMakerHook.test_delete_model_when_not_exist ___________________________________________________________________________
providers/amazon/tests/unit/amazon/aws/hooks/test_sagemaker.py:706: in test_delete_model_when_not_exist
assert ex.response["ResponseMetadata"]["HTTPStatusCode"] == 404
E assert 400 == 404
```
Error in: https://github.com/apache/airflow/actions/runs/25429512640/job/74594810763?pr=66002
The CI ran with moto version `+ moto==5.2.0` which seems to now return a `400` as opposed to `404` earlier. Accept both codes so the test passes against Airflow 3.2.1 compat environments.
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #66473:
URL: https://github.com/apache/airflow/pull/66473#discussion_r3195765102
##########
providers/amazon/tests/unit/amazon/aws/hooks/test_sagemaker.py:
##########
@@ -703,7 +703,7 @@ def test_delete_model_when_not_exist(self):
hook.delete_model(model_name="test")
ex = raised_exception.value
assert ex.operation_name == "DeleteModel"
- assert ex.response["ResponseMetadata"]["HTTPStatusCode"] == 404
+ assert ex.response["ResponseMetadata"]["HTTPStatusCode"] in (400, 404)
Review Comment:
let's add a comment here. otherwise, it's confusing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #66473:
URL: https://github.com/apache/airflow/pull/66473#issuecomment-4390402529
> moto changed the HTTP status code returned for DeleteModel
Do we need to bump min version for moto? I assume after merging this change if CI will fall back to older version we will see errors on the code as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 closed pull request #66473: Fix SageMaker delete model compat test to handle moto status code change
URL: https://github.com/apache/airflow/pull/66473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #66473:
URL: https://github.com/apache/airflow/pull/66473#issuecomment-4392419869
Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #66473:
URL: https://github.com/apache/airflow/pull/66473#discussion_r3195810354
##########
providers/amazon/tests/unit/amazon/aws/hooks/test_sagemaker.py:
##########
@@ -703,7 +703,7 @@ def test_delete_model_when_not_exist(self):
hook.delete_model(model_name="test")
ex = raised_exception.value
assert ex.operation_name == "DeleteModel"
- assert ex.response["ResponseMetadata"]["HTTPStatusCode"] == 404
+ assert ex.response["ResponseMetadata"]["HTTPStatusCode"] in (400, 404)
Review Comment:
Alrighty, handled: [Add comment explaining moto status code compat in delete model test](https://github.com/apache/airflow/pull/66473/commits/09539f8ae97d7519c023624a067e49bf17004c3d)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #66473:
URL: https://github.com/apache/airflow/pull/66473#issuecomment-4389827457
Good to merge but the reference faluire seems in Compat tests, would be good to see CI as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix SageMaker delete model compat test to handle moto status code change [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #66473:
URL: https://github.com/apache/airflow/pull/66473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org