Skip to content

Fix providers tests in main branch with eager upgrades #18040

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 6, 2021
Merged

Fix providers tests in main branch with eager upgrades #18040

merged 1 commit into from
Sep 6, 2021

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Sep 5, 2021 •
edited
Loading

The SQS and DataCatalog were failing tests in main branch because
some recent release of dependencies broke them:

  1. SQS moto 2.2.6 broke SQS tests - the queue url in the 2.2.6+
    version has to start with http:// or https://

  2. DataCatalog part of Google Provider incorrectly imported
    types and broke tests (used beta instad of datacatalog path)

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@potiuk potiuk requested a review from turbaszek as a code owner September 5, 2021 22:37
@potiuk potiuk requested review from mik-laj, eladkal, ashb and kaxil September 5, 2021 22:37
@potiuk
Copy link
Member Author

potiuk commented Sep 5, 2021

Another set of fixes to failing "eager-upgrade" tests in main. Hopefully the last one (still a docs build error remains despite recent fix)

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Sep 5, 2021
@github-actions
Copy link

github-actions bot commented Sep 5, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@uranusjr
Copy link
Member

uranusjr commented Sep 6, 2021

@potiuk Could you add some context how 2.2.6 and 2.2.7 break the tests? This information would be useful in the future when we consider relaxing the version range.

@uranusjr
Copy link
Member

uranusjr commented Sep 6, 2021

Hmm, Sphinx does not seem to like this?

TypeError("unsupported operand type(s) for +: 'SSL_VERIFY_PEER' and 'SSL_VERIFY_FAIL_IF_NO_PEER_CERT'")

@potiuk
Copy link
Member Author

potiuk commented Sep 6, 2021 

TypeError("unsupported operand type(s) for +: 'SSL_VERIFY_PEER' and 'SSL_VERIFY_FAIL_IF_NO_PEER_CERT'")

Yeah, I was loooking in the evening yesterday and tried to pin-point where this one is coming from. If you have any idea, how to approach that one it would be great. I could not find any clue or similar issues.

It looks like some dependency (but I have no idea which one) causes this one. For now I was trying to downgrade each of the dependencies one-by-one to the version that "works" (current constraint) but this is a bit "hit-and-miss" approach and that will take me a while to go through it. Any wild guesses ?

@potiuk
Copy link
Member Author

potiuk commented Sep 6, 2021 •
edited
Loading

@potiuk Could you add some context how 2.2.6 and 2.2.7 break the tests? This information would be useful in the future when we consider relaxing the version range.

Srsly I do not know yet :). When we upgrade to 2.2.6, the sqs tests fail with https://github.com/apache/airflow/runs/3518155510?check_suite_focus=true#step:6:9582

tests/providers/amazon/aws/operators/test_sqs.py::TestSQSPublishOperator::test_execute_success: botocore.exceptions.ClientError: An error occurred (InvalidAddress) when calling the SendMessage operation: The address test is not valid for this endpoint.
tests/providers/amazon/aws/sensors/test_sqs.py::TestSQSSensor::test_poke_message_invalid_filtering: botocore.exceptions.ClientError: An error occurred (InvalidAddress) when calling the SendMessage operation: The address test is not valid for this endpoint.
tests/providers/amazon/aws/sensors/test_sqs.py::TestSQSSensor::test_poke_no_message_failed: botocore.exceptions.ClientError: An error occurred (InvalidAddress) when calling the ReceiveMessage operation: The address test is not valid for this endpoint.
tests/providers/amazon/aws/sensors/test_sqs.py::TestSQSSensor::test_poke_success: botocore.exceptions.ClientError: An error occurred (InvalidAddress) when calling the SendMessage operation: The address test is not valid for this endpoint.

Any help/suggestions/guesses are welcome. I would love to fix those tests rather than limit moto but if we can't find the reason quickly, limiting moto might be a faster way.

@potiuk
Fix providers tests in main branch with eager upgrades
67580c6 
The SQS and DataCatalog were failing tests in main branch because
some recent release of dependencies broke them:

1) SQS moto 2.2.6 broke SQS tests - the queue url in the 2.2.6+
   version has to start with http:// or https://

2) DataCatalog part of Google Provider incorrectly imported
   types and broke tests (used beta instad of datacatalog path)
@potiuk potiuk force-pushed the fix-main-tests-for-providers branch from b9a9f0a to 67580c6 Compare September 6, 2021 08:21
@potiuk
Copy link
Member Author

potiuk commented Sep 6, 2021

Thanks @uranusjr for help! the SQS tests should now be working even with moto >= 2.2.6

@potiuk
Copy link
Member Author

potiuk commented Sep 6, 2021

I will deal with doc builds separately

@potiuk
Copy link
Member Author

potiuk commented Sep 6, 2021

The static checks fail due to .docker.json corruption (another race condition). Merging to test it in main

@potiuk potiuk merged commit bfad233 into apache:main Sep 6, 2021
@potiuk potiuk deleted the fix-main-tests-for-providers branch September 6, 2021 09:04
@potiuk potiuk mentioned this pull request Sep 30, 2021
Closed
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mik-laj mik-laj mik-laj approved these changes

@turbaszek turbaszek Awaiting requested review from turbaszek

@eladkal eladkal Awaiting requested review from eladkal

@ashb ashb Awaiting requested review from ashb

@kaxil kaxil Awaiting requested review from kaxil

Assignees
No one assigned
Labels
full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@potiuk @uranusjr @mik-laj