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

Move plugin config to root.pipeline.[step].settings #464

Merged
merged 11 commits into from
Dec 4, 2021

Conversation

jolheiser
Copy link
Member

@jolheiser jolheiser commented Oct 20, 2021

This PR moves vargs to settings.

More specifically, it moves

pipline:
  something:
    image: my/plugin
    setting1: foo
    setting2: bar

to

pipline:
  something:
    image: my/plugin
    settings:
      setting1: foo
      setting2: bar

Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser jolheiser added enhancement improve existing features breaking will break existing installations if no manual action happens labels Oct 20, 2021
@jolheiser jolheiser added this to the 0.15.0 milestone Oct 20, 2021
@6543
Copy link
Member

6543 commented Oct 20, 2021

If we break with old pipeline config i think we should have a look at drome 1.0 pipeline format ...

Whats even better would be to add a version flag so we can be backwards compatible. Breaking with things for admin is one thing. But breaking with things user use is a other story in my opinion

@6543
Copy link
Member

6543 commented Oct 20, 2021

Idear is if version is not set it's v1

And your breaking changes is v2

Let tje v1 parse stay as is and ad a new one?
reuse code where posible.

@jolheiser
Copy link
Member Author

If we break with old pipeline config i think we should have a look at drome 1.0 pipeline format

We can look at this, but I don't know that it would be good to do in a single PR, however...(see below)

Idear is if version is not set it's v1
And your breaking changes is v2

In order to make the transition a bit easier, maybe we flag version 2 pipeline config format as "unstable" until we finish making changes?
This would allow users to opt-in to version 2 with the understanding that we are still making changes to it.


In terms of this PR

  1. I can make v2 in a separate package so we continue to support v1
  2. Document somewhere the changes from v1->v2
    1. We can add more to these docs as changes are made to v2 format

@6543
Copy link
Member

6543 commented Oct 20, 2021

yes I like your proposal ...

about packages what about
pipeline/frontend/yaml/compiler/v1 -> old stuff
pipeline/frontend/yaml/compiler/v2 -> new stuff
pipeline/frontend/yaml/compiler/common -> shared stuff
?

@jolheiser jolheiser added the wip label Oct 20, 2021
@anbraten
Copy link
Member

anbraten commented Oct 21, 2021

Just as an idea: Could we add support for a config plugin as later versions of drone have it, to use it to convert v1 woodpecker configs to new v2 configs on the fly and then only add code / support for the newest schema in the main server itself?

@jolheiser
Copy link
Member Author

I think at some point we could deprecate v1?
Possibly once v2 is "finalized" we can offer a migratory tool to help people convert?

@anbraten
Copy link
Member

I think at some point we could deprecate v1? Possibly once v2 is "finalized" we can offer a migratory tool to help people convert?

If it is not to complicated to support two schema in parallel as 6543 suggested, then I think we can also add support for both. Just had the feeling it would duplicate a lot of code and adding the config plugin support should be considered at some point anyways.

@6543
Copy link
Member

6543 commented Oct 21, 2021

To prefent duplicated code if posible iproposed a common package

Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser
Copy link
Member Author

jolheiser commented Oct 24, 2021

I looked at this a bit more, and unfortunately it seems it would require a fair amount of copying.

What I have currently isn't ideal, but would work in the interim. Essentially it combines the past behavior with the new.

Any unknown keys are pushed into Vargs, which are then merged into Settings.

I'm not sure how we want to move forward with the format. Maintaining a v1and v2 would likely require a fair amount of copying.


One thing we could possibly do is copy the v1 as-is one time, that is for a "converter" that will move v1 configs to v2

@anbraten anbraten changed the title Move Vargs to Settings Move plugin config to root.pipeline.[step].settings Oct 26, 2021
@anbraten
Copy link
Member

anbraten commented Dec 3, 2021

TODO note: update json schema

@6543
Copy link
Member

6543 commented Dec 3, 2021

@anbraten can you add the schema part to this pull?

@anbraten
Copy link
Member

anbraten commented Dec 4, 2021

@6543 I updated the schema a bit and fixed tests.

@6543 6543 merged commit 71b9179 into woodpecker-ci:master Dec 4, 2021
@6543 6543 removed the wip label Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking will break existing installations if no manual action happens enhancement improve existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants