Skip to content

Change variable $TASK_DEFINITION to $taskDefinition (--task-definition-file option not working)#215

Merged
fillup merged 1 commit intosil-org:developfrom
inokappa:fix-variables-in-createNewTaskDefJson
Feb 26, 2021
Merged

Change variable $TASK_DEFINITION to $taskDefinition (--task-definition-file option not working)#215
fillup merged 1 commit intosil-org:developfrom
inokappa:fix-variables-in-createNewTaskDefJson

Conversation

@inokappa
Copy link
Copy Markdown
Contributor

@inokappa inokappa commented Sep 28, 2020

Hi

When we use --task-definition-file, the content of the file specified by the --task-definition-file option is not used,
because of the following situation.

    if [ $TASK_DEFINITION_FILE == false ]; then
        taskDefinition="$TASK_DEFINITION"
    else
        taskDefinition="$(cat $TASK_DEFINITION_FILE)"
    fi

    # Get a JSON representation of the current task definition
    # + Update definition to use new image name
    # + Filter the def
    if [[ "x$TAGONLY" == "x" ]]; then

      DEF=$( echo "$TASK_DEFINITION" \
            | sed -e 's~"image":.*'"${imageWithoutTag}"'.*,~"image": "'"${useImage}"'",~g' \
            | jq '.taskDefinition' )

Please confirm.

Yohei

@inokappa inokappa changed the title Change variable $TASK_DEFINITION to $taskDefinition Change variable $TASK_DEFINITION to $taskDefinition (--task-definition-file option not working) Sep 29, 2020
@jfranzoi
Copy link
Copy Markdown

jfranzoi commented Feb 7, 2021

Hi, I can confirm as well.

I was just trying to get --task-definition-file working, patched with the same fix, and going to open a PR.. but finally found there was already this one 👍.

Only reminder for others, don't forget that task definition JSON object starts with:
{ "taskDefinition": { ... } }! (I spent almost one hour digging on why it was loading an empty definition..)

@fillup fillup merged commit 993579e into sil-org:develop Feb 26, 2021
@fillup
Copy link
Copy Markdown
Contributor

fillup commented Feb 26, 2021

Thank you @inokappa!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants