Skip to content

remove unneeded TaskLogStreamer binding override#14176

Merged
cryptoe merged 1 commit intoapache:masterfrom
nlippis:remove-task-log-streamer-binding
Apr 27, 2023
Merged

remove unneeded TaskLogStreamer binding override#14176
cryptoe merged 1 commit intoapache:masterfrom
nlippis:remove-task-log-streamer-binding

Conversation

@nlippis
Copy link
Copy Markdown
Contributor

@nlippis nlippis commented Apr 27, 2023

Fix log and report streaming for tasks run by the kubernetes task runner

#14040 added a binding from TaskLogStreamer to TaskLogs. This binding overrode the existing binding from TaskLogStreamer to SwitchingTaskLogStreamer in CliOverlord. The effect bing that calls to get task reports or logs in the overlord api would go directly to check task storage instead of checking the task runner first. Since the binding from TaskLogStreamer to TaskLogs is not needed, we remove it here.


Key changed/added classes in this PR
  • K8sOverlordModule

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Copy link
Copy Markdown
Contributor

@cryptoe cryptoe left a comment

Choose a reason for hiding this comment

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

Changes LGTM!!
Thanks for the contribution @nlippis

@cryptoe cryptoe merged commit 6579c1c into apache:master Apr 27, 2023
clintropolis pushed a commit to clintropolis/druid that referenced this pull request Apr 27, 2023
@clintropolis clintropolis added this to the 26.0 milestone Apr 27, 2023
abhishekagarwal87 pushed a commit that referenced this pull request May 1, 2023
Co-authored-by: Nicholas Lippis <nick.lippis@imply.io>
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