Skip to content

Refactor when parameters flags are parsed#1337

Merged
carolynvs merged 4 commits intogetporter:mainfrom
carolynvs:pr1331
Oct 27, 2020
Merged

Refactor when parameters flags are parsed#1337
carolynvs merged 4 commits intogetporter:mainfrom
carolynvs:pr1331

Conversation

@carolynvs
Copy link
Copy Markdown
Member

@carolynvs carolynvs commented Oct 22, 2020

What does this change

Parse parameters after bundle is loaded

  • ExecuteAction performs any action but uninstall since that operation is performed backwards.
  • Move resolution of parameters later, when we convert the options into cnab arguments, so that we have the bundle definition available
  • Only validate that --param values are in the proper format, PARAM=VALUE

What issue does it fix

Fixes #1320

Notes for the reviewer

I'd appreciate feedback on if this is an improvement or not. I was trying to have us parse parameters once, and move towards reuse with our new BundleAction interface.

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml)

vdice and others added 4 commits October 20, 2020 16:09
Signed-off-by: Vaughn Dice <vadice@microsoft.com>
* ExecuteAction performs any action but uninstall since that operation
is performed backwards.
* Move resolution of parameters later, when we convert the options into
cnab arguments, so that we have the bundle defintion available
* Only validate that --param values are in the proper format,
PARAM=VALUE

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Since validate modifies the action options, we should store the pointer
so that we can retain and changes made.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@vdice
Copy link
Copy Markdown
Member

vdice commented Oct 22, 2020

This looks like a great improvement to me! Definitely in favor of this refactor.

We can add a note that this will close #1320 when ready.

@carolynvs
Copy link
Copy Markdown
Member Author

/azp run porter

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@carolynvs carolynvs marked this pull request as ready for review October 27, 2020 14:20
@carolynvs
Copy link
Copy Markdown
Member Author

Okay, the build is working again so this is ready for review.

Copy link
Copy Markdown
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

LGTM!

@carolynvs carolynvs merged commit 0c97c9d into getporter:main Oct 27, 2020
@carolynvs carolynvs deleted the pr1331 branch October 27, 2020 15:59
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.

Using file type parameters from parameterset on published bundle fails with base64 encoding issue

2 participants