-
Notifications
You must be signed in to change notification settings - Fork 717
Use ECS properties instead of container properties #6270
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: jorgee <[email protected]>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
Nice, let's test using |
@jorgee aws tests are failing |
It seems the tests are failing because of the job definition reuse. The old definition is using containerProperties and the new one is trying to override with ecsProperties. If I am not wrong, the job definition is reused if container name and nf-token is the same, isn't it? So, I think we should include something else to the token to ensure both are using the same container or ecs properties. |
Signed-off-by: jorgee <[email protected]>
Signed-off-by: jorgee <[email protected]>
Token was basically hashing the result of |
Launching some e2e tests |
@jorgee would not this impact also the xpack plugin for amazon? |
The xpack is overriding the |
When testing with xpack I have got this error. Not sure yet if it is related to this change or happening also with the master.
|
It was an xpack gradle problem. Working after applying the fix. |
plugins/nf-amazon/src/main/nextflow/cloud/aws/batch/AwsBatchTaskHandler.groovy
Show resolved
Hide resolved
It seems that something is broken with the e2e test triggering condition. It has not triggered the e2e tests. |
Signed-off-by: jorgee <[email protected]>
The problem was the git commit message was returning a string with escapes due to
I have changed |
Closes #5096
Ports the @tom-seqera implementation in #5391 to AWS SDK v2 implementation.
With the
RegisterJobDefinitionModel
andContainerPropertiesModel
introduced by @pditommaso, these changes shouldn't affect the xpack as the interface it extends remains untouched.