Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

[WIP] Move ClientContext into StreamApp #211

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

manuzhang
Copy link
Contributor

@manuzhang manuzhang commented Aug 5, 2017

R: @huafengw

This is work in process and even doesn't compile (breaking RemoteMaterializerImpl). The motivation is to make user application even simpler from

val context = ClientContext(akkaConfig)
val app = StreamApp(name, context)
...
val runningApplication = context.submit(app)
context.close()

to

val app = StreamApp(name, akkaConfig) // creates ClientContext 
val runningApplication: RunningApplication = app.run() // invokes context.submit(app)

and address the following issues I find

  1. One subtlety in the current way is StreamApp is implicitly converted to StreamApplication for context.submit. It can be broken if a user forgets to import the conversion (and user doesn't know where it is).
  2. I find it difficult to explain the usage of class StreamApp(name: String, system: ActorSystem, userConfig: UserConfig, private val graph: Graph[Op, OpEdge]). We'd better make the constructor private if it's not intended for users.
  3. Both RunningApplication and ClientContext have a askAppMaster method to query application status. How should their roles be divided ?
  4. RunningApplication has no ScalaDoc.

One downside about the new way is we can't close ClientContext and its underlying ActorSystem. Is it a big problem ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant