Fix Trustchain failing from command line #57
Conversation
…so that we need not have a UI dispatcher when running on command line.
|
Would anyone please be so kind as to consider reviewing and merging this PR or stating what should be changed about it? The functionality in this PR is critical for my project (at least, to me it currently appears that way) and I would like to work from the master branch if that is possible. |
|
@KoningR I will merge this today, can you fix the conflicts first? |
| @@ -64,7 +61,7 @@ abstract class Community : Overlay { | |||
| network.blacklist.addAll(DEFAULT_ADDRESSES) | |||
|
|
|||
| job = SupervisorJob() | |||
| scope = CoroutineScope(Dispatchers.Main + job) | |||
| scope = CoroutineScope(dispatcher + job) | |||
There was a problem hiding this comment.
(4/4) This will now be Dispatchers.Default as opposed to being Dispatchers.Main in the demo application.
There was a problem hiding this comment.
This is precisely what we want to be doing. The reason this issue exists is because a command line application does not (necessarily) have a Main dispatcher. The Main dispatcher is "confined to the Main thread operating with UI objects.". Please refer to issue #57, where I've described the issue more explicitly. By making the dispatcher an optional argument, existing applications should not break. But future console applications should refrain from using a Main dispatcher.
| @@ -78,7 +79,7 @@ class Application { | |||
| ), walkerInterval = 1.0) | |||
|
|
|||
| val ipv8 = IPv8(endpoint, config, myPeer) | |||
| ipv8.start() | |||
| ipv8.start(dispatcher) | |||
There was a problem hiding this comment.
(1/4) Now we're passing Dispatchers.Default.
| @@ -31,7 +31,7 @@ class IPv8( | |||
| return overlays[T::class.java] as? T | |||
| } | |||
|
|
|||
| fun start() { | |||
| fun start(dispatcher: CoroutineDispatcher = Dispatchers.Main) { | |||
There was a problem hiding this comment.
(2/4) Resulting in this not being Dispatchers.Main
| @@ -51,7 +51,7 @@ class IPv8( | |||
| overlay.endpoint = endpoint | |||
| overlay.network = network | |||
| overlay.maxPeers = overlayConfiguration.maxPeers | |||
| overlay.load() | |||
| overlay.load(dispatcher) | |||
There was a problem hiding this comment.
(3/4) We're now passing Dispatchers.Default.
…fix-trustchain-coroutines-jvm � Conflicts: � ipv8/src/main/java/nl/tudelft/ipv8/Community.kt
|
The rewrite of the |
|
@KoningR your changes appear to break all EVA tests. Master: This branch: |
…fix-trustchain-coroutines-jvm
|
I am now using ipv8 directly instead of trustchain (for unrelated reasons). Thus, this problem is not relevant to my project anymore, nor am I involved with the super app for the time being. Time constraints disallow me to spend further time on this. I am leaving the issue and branch open but am closing this PR. |
A (quite naive) solution to fix #56 . At the moment, I am unsure if this is the best way of fixing this, but I am hoping the project maintainer can comment.