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

feat: Provide values of withItems maps as JSON in {{item}}. Fixes #1905 #1906

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

markterm
Copy link
Contributor

@markterm markterm commented Jan 6, 2020

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234". Why? for the release notes.
  • Optional. I've added My organization is added to the README.
  • I've signed the CLA and required builds are green.

@markterm markterm changed the title feat: Provide values of withItems maps as JSON in {{item}}. Fixed #1905 feat: Provide values of withItems maps as JSON in {{item}}. Fixes #1905 Jan 6, 2020
@codecov
Copy link

codecov bot commented Jan 6, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@59a1906). Click here to learn what that means.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1906   +/-   ##
=========================================
  Coverage          ?   11.17%           
=========================================
  Files             ?       35           
  Lines             ?    23541           
  Branches          ?        0           
=========================================
  Hits              ?     2630           
  Misses            ?    20573           
  Partials          ?      338
Impacted Files Coverage Δ
workflow/controller/operator.go 56.27% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59a1906...0e1fcb8. Read the comment docs.

@markterm markterm force-pushed the withitems-maps-as-json branch from ba62bbe to 14c004f Compare January 6, 2020 13:09
Copy link
Member

@simster7 simster7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@simster7 simster7 self-assigned this Jan 6, 2020
@simster7 simster7 merged commit 5d7e918 into argoproj:master Jan 7, 2020
@sachsbl
Copy link

sachsbl commented Feb 24, 2020

This broke my workflow. Previously there was an extra layer of json deserialization applied to the items in the list, as compared to current.

@simster7
Copy link
Member

@sachsbl @salanki Can you guys provide sample workflows, if possible

@sachsbl
Copy link

sachsbl commented Feb 24, 2020

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: workflow-template
spec:
  arguments:
    parameters:
    - name: event-payload
      value: ""
  ttlSecondsAfterFinished: 300
  serviceAccountName: workflow
  entrypoint: workflow-template
  parallelism: 4
  templates:
  - name: workflow-template
    steps:
    - - name: job-creator
        template: job-creator
    - - name: run-job
        template: run-job
        arguments:
          parameters:
          - name: job
            value: "{{item}}"
        withParam: "{{steps.job-creator.outputs.parameters.jobs-list}}"
  - name: job-creator
    podSpecPatch: |
      containers:
        - name: main
          resources:
            requests:
              cpu: 1
              memory: 14336Mi
    container:
      image: workflow
      imagePullPolicy: Always
      command: [python3, "job_creator.py"]
    outputs:
      parameters:
      - name: jobs-list
        valueFrom:
          path: /tmp/jobs_list.json
  - name: run-job
    podSpecPatch: |
      containers:
        - name: main
          resources:
            requests:
              cpu: 2
              memory: 4048Mi
    inputs:
      parameters:
      - name: job
    container:
      image: workflow
      imagePullPolicy: Always
      command: [python3, "process_job.py"]
      args: ["{{inputs.parameters.job}}"]
      volumeMounts:
        - name: nfs-volume
          mountPath: /efs/

@sachsbl
Copy link

sachsbl commented Feb 24, 2020

that workflow still works. However, the value of "{{item}}" passed to the job now has one layer less of json encoding. Honestly this felt like a bug before, and the current behavior makes more sense. It did break though :-)

@salanki
Copy link
Contributor

salanki commented Feb 24, 2020

Please see: #2248

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.

4 participants