refactor!: Remove cross-region CWL routing from log config options - #1050
Conversation
08ec7cf to
e0083f3
Compare
This reverts commit a3ce25c (aws-deadline#997), keeping the log_constants.py extraction so the log config option key constants stay in a leaf module and do not reintroduce a cyclic import. This code is redundant: workers already route session logs to the home region by default, so the region handling in the log config options is not needed. BREAKING CHANGE: the worker agent no longer honors a region in the log configuration options. Signed-off-by: MarinMaksutaj <19916629+MarinMaksutaj@users.noreply.github.com>
e0083f3 to
4c9b055
Compare
| @@ -4,4 +4,3 @@ | |||
|
|
|||
| LOG_CONFIG_OPTION_GROUP_NAME_KEY = "logGroupName" | |||
| LOG_CONFIG_OPTION_STREAM_NAME_KEY = "logStreamName" | |||
There was a problem hiding this comment.
Release tooling will silently drop this breaking change from the CHANGELOG.
The commit subject is revert!: ..., but revert is not in allowed_tags in pyproject.toml:241-252. python-semantic-release ignores commits whose type is not in allowed_tags, so this commit will not produce a CHANGELOG entry and (on its own) will not trigger a version bump.
This is not hypothetical — every prior revert: commit in this repo is absent from CHANGELOG.md:
revert: Revert "chore: remove MANIFEST_REPORTING_FEATURE code (#855)" (#864)revert: Revert "test(debug): add e2e pytest session timeout ... (#793)" (#813)
That matters more than usual here because the feature being removed was shipped and announced. CHANGELOG.md:20 (v0.31.0) says:
Cross-region CloudWatch Logs routing now uses the region specified in log configuration options, enabling proper log delivery for multi-region fleet configurations. (#997)
So as things stand, 0.31.0 advertises the feature, and the release that removes it says nothing — leaving users who configured a region log option with no signal that the agent has stopped honoring it. The BREAKING CHANGE: trailer in the commit body does not rescue this, since the commit type is filtered out before the trailer is considered.
Options, roughly in order of least churn:
- Retype the subject to an allowed tag that carries the semantics, e.g.
fix!:orrefactor!:, keeping theBREAKING CHANGE:trailer so it lands in the CHANGELOG. - Add
reverttoallowed_tags(and probablypatch_tags) if reverts should be first-class going forward — this also fixes the general case rather than just this PR.
Worth confirming against your release process which you prefer; either way the current subject means the removal ships unannounced.
|
Could you update PR description to follow the PR template. |
What was the problem/requirement? (What/Why)
#997 (a3ce25c) added cross-region CloudWatch Logs routing: the worker agent reads a region from the log configuration options and routes session logs to that region. This code is redundant: workers already route session logs to the home region by default today, so the extra region handling is not needed.
What was the solution? (How)
Remove the cross-region routing added in #997. This is mostly a revert, but not a pure one, which is why it is typed refactor! rather than revert:
Removed (from #997):
Kept (from #997):
What is the impact of this change?
The worker agent no longer honors a region in the log configuration options; CloudWatch Logs clients use the default routing.
How was this change tested?
Unit tests (3047 passed), ruff check, ruff format, and mypy all pass.
Was this change documented?
No documentation changes needed; the removed option was not documented.
Is this a breaking change?
Yes.
BREAKING CHANGE: the worker agent no longer honors a region in the log configuration options; CloudWatch Logs clients use the default routing.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.