-
Notifications
You must be signed in to change notification settings - Fork 224
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
Use EnvironmentFile instead of Environment #1762
Use EnvironmentFile instead of Environment #1762
Conversation
I added the parameter for choosing the existing way in this commit 27d1ae3. |
Sorry for very late reply.
|
2d40618
to
af9aa14
Compare
fix document extract process about uploading environmentFile to S3 as a method
af9aa14
to
fe6f8bf
Compare
@yoyama
It makes sense to me, so I modified my implementation with this commit fe6f8bf. I left
I also dealt with this concern with the commit above. I extracted a process about uploading environmentFile to S3 as another method from If there are any other concerns, please let me know about those. |
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.
Sorry for the late reply.
I agree with adding ecs.<name>.use_environment_file
to the system property. Actually, this property can also be set from the task config, but since the default is false and does not affect the behavior, it should be fine.
I have left some minor comments, I would appreciate it if you take a look at them when you get a chance.
digdag-standards/src/main/java/io/digdag/standards/command/EcsCommandExecutor.java
Outdated
Show resolved
Hide resolved
digdag-standards/src/main/java/io/digdag/standards/command/EcsCommandExecutor.java
Outdated
Show resolved
Hide resolved
digdag-standards/src/main/java/io/digdag/standards/command/EcsCommandExecutor.java
Outdated
Show resolved
Hide resolved
...g-standards/src/main/java/io/digdag/standards/command/ecs/TemporalProjectArchiveStorage.java
Outdated
Show resolved
Hide resolved
ab567e4
to
9eb7d7a
Compare
fix storage type with s3 remove unnecessary variable
9eb7d7a
to
02030bd
Compare
@szyn |
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.
Looks good to me. Thank you for your contribution.
Overview
fix #1749
I made a change that environment file is used when passing the values of environment to the container instead of environment, that is the way to add them to the container one by one.
This change will be effective to avoid failing an execution of ECS Run Task because a maximum size of characters of ECS container overrides is 8192 and this limit includes environments defined by the current implementation.
it will be helpful especially when executing
sh>:
operator. In addition to variables defined in_env
, variables defined in_export
are also passed to the container unlikepy>:
andrb>:
so the size of environment tends to be larger as the development progresses.The change points