Skip to content

Conversation

@gopidesupavan
Copy link
Member

@gopidesupavan gopidesupavan commented Sep 28, 2025

Adding Remote logging tests to E2E test.

Why:

Currently we dont have a way to test remote logging, all that we are doing so far mocking handlers and we have seen recent times remote logging broken many times.

This is to add remote logging e2e test that replicates real environment with AWS using localstack. This test is remote logging with S3. for other remote logging it can be extended later.


^ 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 airflow-core/newsfragments.

@gopidesupavan
Copy link
Member Author

Tests needs to be added, not sure its tricky part where to add is it in airflow-core or task-sdk 🤔

@ashb
Copy link
Member

ashb commented Sep 29, 2025

Tests needs to be added, not sure its tricky part where to add is it in airflow-core or task-sdk 🤔

Neither, it's integration between the two

@gopidesupavan
Copy link
Member Author

Tests needs to be added, not sure its tricky part where to add is it in airflow-core or task-sdk 🤔

Neither, it's integration between the two

hehe correct :)

@ashb ashb changed the title Use in-process-api-server for connection when task sdk context not available Fix loading Connections in API server in order to read remote logs Sep 29, 2025
@gopidesupavan
Copy link
Member Author

image

@gopidesupavan
Copy link
Member Author

have added a full e2e integration test with local stack,

@gopidesupavan
Copy link
Member Author

I am not sure if you wanted me to create separate PR for testing update i am happy to do that, i have added it part of this because we are trying to fix the remote log issue, i feel its make sense to add .

@gopidesupavan gopidesupavan force-pushed the fix-connection-handling branch 2 times, most recently from a698cd0 to 2222c11 Compare September 30, 2025 00:46
@gopidesupavan
Copy link
Member Author

verified on real env:

image

@gopidesupavan gopidesupavan force-pushed the fix-connection-handling branch 4 times, most recently from fadc410 to 89570d0 Compare September 30, 2025 11:30
@gopidesupavan
Copy link
Member Author

I think this is ready for review 2 tests are failing not related to this, am trying to look them.

@gopidesupavan
Copy link
Member Author

raised PR here for failure tests #56270

@gopidesupavan gopidesupavan force-pushed the fix-connection-handling branch from 2b30a66 to 4558e68 Compare October 3, 2025 09:32
@kaxil
Copy link
Member

kaxil commented Oct 13, 2025

Bump on this one @gopidesupavan :) If you can address the comments, we can release 3.1.1 sooner with it :)

@gopidesupavan gopidesupavan force-pushed the fix-connection-handling branch 2 times, most recently from 785544a to 280a597 Compare October 14, 2025 08:53
@jason810496 jason810496 self-requested a review October 14, 2025 13:43
@gopidesupavan gopidesupavan changed the title Fix loading Connections in API server in order to read remote logs Add e2e test for remote logging Oct 14, 2025
@gopidesupavan gopidesupavan marked this pull request as draft October 14, 2025 20:24
@gopidesupavan
Copy link
Member Author

Making changes to this pr only to add e2e tests for remote logging, as the issue is fixed by @kaxil in #56602

@kaxil
Copy link
Member

kaxil commented Oct 14, 2025

Making changes to this pr only to add e2e tests for remote logging, as the issue is fixed by @kaxil in #56602

Thanks @gopidesupavan

@gopidesupavan gopidesupavan force-pushed the fix-connection-handling branch 2 times, most recently from a202686 to cb5c9dc Compare October 14, 2025 21:01
@gopidesupavan gopidesupavan marked this pull request as ready for review October 15, 2025 06:22
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Nice. I like the composed docker-compose file :)

@potiuk potiuk force-pushed the fix-connection-handling branch from cb5c9dc to 92f5172 Compare October 15, 2025 12:56
@potiuk
Copy link
Member

potiuk commented Oct 15, 2025

Rebased to account for fixed task-sdk integration tests (just in case).

@gopidesupavan
Copy link
Member Author

Nice. I like the composed docker-compose file :)

yeah :)

@gopidesupavan gopidesupavan merged commit 66d5e72 into apache:main Oct 15, 2025
153 of 154 checks passed
@gopidesupavan gopidesupavan deleted the fix-connection-handling branch October 15, 2025 18:10
@kaxil
Copy link
Member

kaxil commented Oct 15, 2025

Awesome, thanks @gopidesupavan

snreddygopu pushed a commit to Teradata/airflow that referenced this pull request Oct 16, 2025
* Prefetch remote log connection id for api server in order to read remote logs

* fix docker compose file path

* Fixup tests

* Add test with mock_aws

* Fixup test

* Extend quick start docker with localstack

* remove comment

* add test connection

* fix static checks

* Add only e2e tests for remote logging
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
* Prefetch remote log connection id for api server in order to read remote logs

* fix docker compose file path

* Fixup tests

* Add test with mock_aws

* Fixup test

* Extend quick start docker with localstack

* remove comment

* add test connection

* fix static checks

* Add only e2e tests for remote logging
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
* Prefetch remote log connection id for api server in order to read remote logs

* fix docker compose file path

* Fixup tests

* Add test with mock_aws

* Fixup test

* Extend quick start docker with localstack

* remove comment

* add test connection

* fix static checks

* Add only e2e tests for remote logging
TyrellHaywood pushed a commit to TyrellHaywood/airflow that referenced this pull request Oct 22, 2025
* Prefetch remote log connection id for api server in order to read remote logs

* fix docker compose file path

* Fixup tests

* Add test with mock_aws

* Fixup test

* Extend quick start docker with localstack

* remove comment

* add test connection

* fix static checks

* Add only e2e tests for remote logging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants