-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Big refactor #12
Big refactor #12
Conversation
- use java library - remove non used code and config Signed-off-by: Jorge Aguilera <jagedn@gmail.com>
I would like to include more tests in order to cover specific situations by the moment I've included an unique test to run a "full pipeline" using a mockWebServer |
@jagedn very nice refactor 👍 looking much more readable now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jorge,
Thanks for this big "merge" PR - the codebase does look much better!
I have made some minor comments and requests, but we can also address them in a later PR as well.
Note to future readers:
- We've made the decision to still explore the OpenAPI spec generated Java SDK
- The library we're using is published here on Clojars https://clojars.org/org.clojars.abhi18av/nomad-client-java
plugins/nf-nomad/src/main/nextflow/nomad/executor/NomadScriptLauncher.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-nomad/src/main/nextflow/nomad/executor/NomadService.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-nomad/src/main/nextflow/nomad/executor/NomadService.groovy
Outdated
Show resolved
Hide resolved
* @author Jorge Aguilera <jagedn@gmail.com> | ||
*/ | ||
|
||
class MockScriptRunner extends ScriptRunner { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagedn , for future - I think that it'd be great to have some latency option within the mock server given that the real-world cluster network might not be reliable. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good idea
with mock server we can simulate lot of responses and test all this kind of situations
Signed-off-by: Jorge Aguilera <jagedn@gmail.com>
This is a refactor of the current project due we're mixing API lib + custom http
It removes lot of comments and unnecessary (by the moment) code