CloudWatch task handler doesn't fall back to local logs when Amazon CloudWatch logs aren't found#27564
Merged
potiuk merged 2 commits intoapache:mainfrom Nov 11, 2022
Merged
Conversation
boring-cyborg
bot
added
area:logging
area:providers
provider:amazon
Contributor
Author
|
@vincbeck |
hankehly
commented
Nov 9, 2022
| [{"end_of_log": True}], | ||
| ) | ||
|
|
||
| def test_read_wrong_log_stream(self): |
Contributor
Author
There was a problem hiding this comment.
I deleted these 2 test cases for the following reasons.
- They overlap with the one I added (testing for generic failure to fetch remote logs)
- AWS error messages are now propagated to the task log. These strings could change in the future and break our unit tests.
hankehly
commented
Nov 9, 2022
| start_from_head=True, | ||
| ) | ||
| return "\n".join(self._event_to_str(event) for event in events) | ||
| except Exception: |
Contributor
Author
There was a problem hiding this comment.
I am propagating the AWS error message to the task log. I think this will be useful for debugging IAM/connection issues.
hankehly
commented
Nov 9, 2022
| f"*** Reading local file: {self.local_log_location}/{self.remote_log_stream}\n" | ||
| ) | ||
| assert log == expected_log | ||
| expected_log_pos = 26 + len(self.local_log_location) + len(self.remote_log_stream) |
Contributor
Author
There was a problem hiding this comment.
Log position seems irrelevant in the context of the CloudWatch handler because we fetch/return all logs in one request. I'm following the example I found in the GCS task handler here.
potiuk
approved these changes
Nov 11, 2022
Contributor
|
🚀 |
Adityamalik123
pushed a commit
to Adityamalik123/airflow
that referenced
this pull request
Nov 12, 2022
…loudWatch logs aren't found (apache#27564)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #27396
Summary
The CloudWatch task log handler does not default to local logs (like the documentation says it should) when remote logs are unavailable. This PR fixes the issue.
Todo
Manual check
Procedure
echo "test" >> {log file path}Expectations
Screenshot