Skip to content
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

add --launch-container option in launch command #444

Merged
merged 11 commits into from
Aug 23, 2024

Conversation

munishchouhan
Copy link
Member

This PR will add --launch-container option in launch command

@munishchouhan munishchouhan linked an issue Aug 21, 2024 that may be closed by this pull request
@munishchouhan munishchouhan marked this pull request as draft August 21, 2024 14:31
@munishchouhan munishchouhan self-assigned this Aug 21, 2024
@pditommaso
Copy link
Contributor

ok, let's sync with @tcrespog for the sdk

@munishchouhan
Copy link
Member Author

@munishchouhan
Copy link
Member Author

PR to add in sdk seqeralabs/tower-sdk-gencode#8

this SDK PR is not correct, checking with @JaimeSeqLabs to fix it

@munishchouhan
Copy link
Member Author

SDK pr has been fixed

@munishchouhan
Copy link
Member Author

@tcrespog is there recent chnages in GoogleBatchPlatform?

Tests are failing for it with new tower sdk


org.opentest4j.AssertionFailedError: expected: <> but was: <
 ERROR: The program tried to reflectively invoke method public java.lang.String io.seqera.tower.model.GoogleBatchConfig.getNextflowConfig() without it being registered for runtime reflection. Add it to the reflection metadata to solve this problem. See https://www.graalvm.org/latest/reference-manual/native-image/metadata/#reflection for help.


 ERROR: Connection error. Check the connection using the command 'tw info'.
>
	at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141)
	at app//io.seqera.tower.cli.BaseCmdTest.assertOutput(BaseCmdTest.java:183)
	at app//io.seqera.tower.cli.computeenvs.platforms.GoogleBatchPlatformTest.testAdd(GoogleBatchPlatformTest.java:61)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at [email protected]/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

@tcrespog
Copy link
Contributor

is there recent chnages in GoogleBatchPlatform?

Yes, in In GoogleBatchConfig and others: https://github.com/seqeralabs/platform/pull/6691.

@JaimeSeqLabs
Copy link
Contributor

JaimeSeqLabs commented Aug 22, 2024

Tests are failing for it with new tower sdk

The program tried to reflectively invoke method public java.lang.String io.seqera.tower.model.GoogleBatchConfig.getNextflowConfig() without it being registered for runtime reflection

I'm assuming the tests failing here are the native ones.
This is most likely because we need to manually add the new method signature to the reflect-config.json file.
(probably more methods that GraalVM missed in the last update)

@munishchouhan munishchouhan marked this pull request as ready for review August 22, 2024 10:55
conf/reflect-config.json Outdated Show resolved Hide resolved
@pditommaso
Copy link
Contributor

pditommaso commented Aug 22, 2024

Was the --launch-container option tested?

@munishchouhan
Copy link
Member Author

Was the --launch-container option tested?

I am not sure, how to test it?

@JaimeSeqLabs
Copy link
Contributor

JaimeSeqLabs commented Aug 22, 2024

  1. Run a local instance of Seqera Platform
  2. Point tw-cli to the local instance: export TOWER_API_ENDPOINT=http://localhost:8000/api
  3. Do a launch using the new parameter: tw --insecure launch ... --launch-container ...

@pditommaso
Copy link
Contributor

pditommaso commented Aug 22, 2024

This option allows you to change the nextflow launcher used by Platform when running a workflow. Use for example quay.io/seqeralabs/nf-launcher:j17-24.07.0-edge.

You may want to use https://cloud.stage-seqera.io/ for sake of simplicity and run this "workflow" https://github.com/pditommaso/nf-sleep

When running it should show 24.07.0 as nextflow version.

Screenshot 2024-08-22 at 18 33 25

(or use your local platform as Jaime is suggesting)

@munishchouhan
Copy link
Member Author

munishchouhan commented Aug 22, 2024

Ok finally my pipeline runs and i can test it successfully
client:

% ./build/native/nativeCompile/tw  launch https://github.com/pditommaso/nf-sleep  \
--launch-container quay.io/seqeralabs/nf-launcher:j17-24.07.0-edge
pipeline_run

@pditommaso
Copy link
Contributor

Excellent.

@pditommaso pditommaso merged commit 9b875ee into master Aug 23, 2024
12 checks passed
@munishchouhan munishchouhan deleted the 436-add-launch-container-option-to-launch-command branch August 23, 2024 08:01
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.

Add --launch-container option to launch command
4 participants