Feature/instanceregistry #20
Conversation
ghost
assigned
Codecov Report
@@ Coverage Diff @@
## develop #20 +/- ##
======================================
Coverage 0% 0%
======================================
Files 6 10 +4
Lines 500 612 +112
Branches 4 15 +11
======================================
- Misses 500 612 +112
Continue to review full report at Codecov.
|
johannesduesing
left a comment
johannesduesing
left a comment
There was a problem hiding this comment.
This PR introduces some code style issues (that's why the one check is failing), namely there are some usages of .get on optional types. Please replace them with .getOrElse (have a look how it was done in the last commit of this PR delphi-hub/delphi-crawler#24) to keep the code quality up.
| val f = (client.execute { | ||
| nodeInfo() | ||
| } map { i => { | ||
| if(configuration.usingInstanceRegistry) InstanceRegistry.sendMatchingResult(true, configuration) |
There was a problem hiding this comment.
As in the other PR: I would like to move this condition into the method.
| Success(configuration) | ||
| } | ||
| } recover { case e => { | ||
| if(configuration.usingInstanceRegistry) InstanceRegistry.sendMatchingResult(false, configuration) |
|
This PR still has some open review comments. |
|
In this PR, are there updates for me to review? Is it still work-in-progress? |
|
Hello Ben, please review the request and let me know if any changes are required. |
|
@johannesduesing We also need your approval here. Please have a look. |
|
I'm happy with the changes, the PR looks good to me. Snyk reports 2 vulerabilities to DoS attacks trough spray-json, do you know if we can do something about that @bhermann ? |
bhermann
dismissed stale reviews from johannesduesing and themself
via
cdad67c
Changes for Instance registry. Please let me know for any changes