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

azp: Expected a mapping error for empty each loop #222

Closed
bryanbcook opened this issue Oct 2, 2023 · 4 comments · Fixed by #223
Closed

azp: Expected a mapping error for empty each loop #222

bryanbcook opened this issue Oct 2, 2023 · 4 comments · Fixed by #223
Labels
bug Something isn't working

Comments

@bryanbcook
Copy link

The following syntax is valid in the azure-pipelines preview API

parameters:

- name: objectArray
  type: object
  default:
  - name: one
    options:
    - a
    - b
  - name: two
    # does not specify options

stages:
- ${{ each item in parameters.objectArray }}:
  - stage: ${{ item.name }}
    jobs:
    - job: ${{ item.name }}
      steps:
      - checkout: none
      - ${{ each option in item.options }}: # errors when options is null
        - bash: echo '${{ option }}'
          displayName: 'Option ${{ option }}'

image

However, when run, I get error The template is not valid. pipeline.yml (Line: 19, Col: 9): Expected a sequence, or a mapping, got:

I believe the issue is in the TemplateUnraveler:425, but not sure where to start.

@ChristopherHX
Copy link
Owner

ChristopherHX commented Oct 2, 2023

Maybe add a

if(!(res is NullToken)) {
    m_context.Error(eachExpressionState.Value, $"Expected a sequence, or a mapping, got: {res?.ToString() ?? "null"}");
}

guard.
This should then just remove the each token from the document.

AFAIK only m_context.Error will trigger the error condition. The parser of actions/runner has been written to continue on errors to provide more error information in a single pass.

EDIT Not that easy to fix than I thought

@ChristopherHX
Copy link
Owner

Thank you for reporting these bugs, it's really helpful to expand the testcases with problems of real world template files with my azure pipelines template engine.

I have created a patch for this.

@bryanbcook
Copy link
Author

I'm glad you were able to figure this out! I'll see what I can do to add a few more integration tests.

Unfortunately, I'm still having issues getting this up and running locally in my own environment. I can log an issue with what I've currently attempted.

As previously expressed, my principal interest is being able to validate the pipelines locally before pushing the commits to origin. I'm not sure that I would want my team to use the emulator to perform deployments locally as we'd lose a lot of traceability. My ideal scenario would be to run the yaml verification without having to spin up a web-server. For example, extracting the ObjectTemplating SDK capabilities into a Visual Studio Code extension that can perform the template parsing to validate that the syntax is valid.

@ChristopherHX
Copy link
Owner

ChristopherHX commented Oct 3, 2023

Unfortunately, I'm still having issues getting this up and running locally in my own environment. I can log an issue with what I've currently attempted.

Yes please do that.

Currently you can use the list flag / command to only validate the template, but not executing them

Runner.Client --event azpipelines --interactive --parallel 0
list

The --interactive avoid restarting the server, so reducing latency
--parallel 0 avoids spinning up any agent,

For example, extracting the ObjectTemplating SDK capabilities into a Visual Studio Code extension that can perform the template parsing to validate that the syntax is valid.

I have already had that idea, but didn't get to implement it. I have created the Azure Pipeliens Expand Webpage #173 instead, due to better tooling from the dotnet sdk at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants