Skip to content

SAMZA-2294: LocalApplicationRunner support for missing coordinator stream - #1130

Merged
xinyuiscool merged 1 commit into
apache:masterfrom
dnishimura:samza-2294-lar-no-coordinator-stream
Aug 7, 2019
Merged

xinyuiscool merged 1 commit into
apache:masterfrom
dnishimura:samza-2294-lar-no-coordinator-stream

Conversation

@dnishimura

Copy link
Copy Markdown
Contributor

Certain standalone configurations do not define a job.coordinator.system because a coordinator stream is not necessary. For example, in PassthroughJobCoordinator configured jobs or integration test jobs might not have this defined. This PR allows support for such scenarios.

Added additional unit tests and verified by @xinyuiscool with his testing.
@xinyuiscool - Please take a look when you get a chance.

@xinyuiscool xinyuiscool left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks for the quick fix!

@shanthoosh shanthoosh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@shanthoosh

Copy link
Copy Markdown
Contributor

@dnishimura
Just curious. Please correct me if I'm wrong.

Do we know why TestRunner integration tests did not fail before this fix, since TestRunner uses both LocalApplicationRunner and PassThroughCoordinator?

@dnishimura

dnishimura commented Aug 7, 2019 •

Copy link
Copy Markdown
Contributor Author

@dnishimura
Just curious. Please correct me if I'm wrong.

Do we know why TestRunner integration tests did not fail before this fix, since TestRunner uses both LocalApplicationRunner and PassThroughCoordinator?

@shanthoosh
I have to verify, but I think it had job.coordinator.system defined.

@xinyuiscool
xinyuiscool merged commit affded2 into apache:master Aug 7, 2019
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.

3 participants