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

Add codestart data option in CLI and Maven plugin #28372

Closed
ia3andy opened this issue Oct 4, 2022 · 26 comments · Fixed by #29644
Closed

Add codestart data option in CLI and Maven plugin #28372

ia3andy opened this issue Oct 4, 2022 · 26 comments · Fixed by #29644
Assignees
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/codestarts area/maven good first issue Good for newcomers kind/enhancement New feature or request
Milestone

Comments

@ia3andy
Copy link
Contributor

ia3andy commented Oct 4, 2022

Description

It would be nice extension codestart to have user customizable data. This is already possible in codestarts but not yet connected in the tooling.

It would behave like this:

quarkus create app -x=my-ext -d=my-ext-codestart.foo.bar=hello,my-ext-codestart.foo.hello=andy

The my-ext-codestart prefix is required by the codestart system to know the data is targeting this particular codestart data (and avoid conflicts)

The default value is in the codestart.yml:

name: my-ext-codestart
language:
  base:
    data:
      foo:
        bar: baz
        hello: world

FYI: I created this issue because @gsmet has a need for this.

@ia3andy ia3andy added the kind/enhancement New feature or request label Oct 4, 2022
@quarkus-bot quarkus-bot bot added area/cli Related to quarkus cli (not maven/gradle/etc.) area/codestarts area/maven labels Oct 4, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 4, 2022

/cc @ebullient, @maxandersen, @quarkusio/devtools

@ia3andy ia3andy added the good first issue Good for newcomers label Oct 4, 2022
@arpitbhardwaj
Copy link

Let me know if i pick it up and start working on this.

@ebullient
Copy link
Contributor

ebullient commented Oct 7, 2022

Does that mean you have to know each individual codestart's requirements to make this useful? There is already a way to pass in initial configuration with existing options. This seems like something that is useful if you write codestarts frequently, but is difficult to document (where the goal of the CLI, e.g., is to ensure it is generally useful without having to go look at docs).

@gsmet
Copy link
Member

gsmet commented Oct 7, 2022

The idea is to be able to pass some additional data for a specific extension codestart. AFAIK there's no way to do that right now.
And in the case of the GitHub Action extension, I'd like to have the ability to pass the GitHub repository where the action will be hosted.

@ebullient
Copy link
Contributor

ebullient commented Oct 7, 2022

yes.. can't that be passed in using the existing config mechanism? Something like --app-config=whatever.github.repo=gsmet/awesomesauce?

@gsmet
Copy link
Member

gsmet commented Oct 10, 2022

It's not a config property I want. It's something that I can actually use in the templates when generating the project.

@gsmet
Copy link
Member

gsmet commented Oct 10, 2022

@arpitbhardwaj you can have a look if you can figure out where to do this.

@gsmet
Copy link
Member

gsmet commented Oct 11, 2022

This might give you a good idea about where to poke: #28325

@maxandersen
Copy link
Contributor

@ia3andy would it not "just work" if you use -D to set properties today?

@maxandersen
Copy link
Contributor

@gsmet I thought app config was available to codestarts ?

@gsmet
Copy link
Member

gsmet commented Oct 11, 2022

I don't want to push these things to the app config.

@maxandersen
Copy link
Contributor

but -D are just system properties - shouldn't be ending up in you generated app config.

@maxandersen
Copy link
Contributor

i.e.

quarkus create app -x=my-ext -Dmy-ext-codestart.foo.bar=hello -Dmy-ext-codestart.foo.hello=andy

should be possible to make work.

@gsmet
Copy link
Member

gsmet commented Oct 11, 2022

So that could work but is there a way to access system properties from the Qute template directly? I might be missing something obvious.

@ebullient
Copy link
Contributor

-- we do set the system properties here..

We could also set them as template values by calling this:

protected void setValue(String name, Object value) {

@gsmet
Copy link
Member

gsmet commented Oct 11, 2022

But why would we do that instead of something cleaner with the data thing?

@maxandersen
Copy link
Contributor

We already use -D to pass an arbitrary keys to maven/gradle/jbang so not sure how much value it brings to add yet-another open-ended key/value command option?

@ia3andy
Copy link
Contributor Author

ia3andy commented Oct 14, 2022

Ok, my concern using plain system properties is that we would have to find a way to filter them before passing them as data to codestarts if we don't want to have weird behaviors.

In the top level where we pass the data, we don't know how it will be consumed by codestarts.

@ia3andy
Copy link
Contributor Author

ia3andy commented Oct 14, 2022

IMO we should reproduce the same syntax as for config on another named option (data or whatever), parse it using the same code as the data and passing the map to codestart data.

We need a new key and setter in the CreateProject for this (like we do for config):
https://github.com/quarkusio/quarkus/blob/main/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/commands/CreateProject.java#L43

Unlike the config this data needs to be directly passed to the builder here:
https://github.com/quarkusio/quarkus/blob/main/independent-projects/tools/devtools-common/src/main/java/io/quarkus/devtools/commands/handlers/CreateProjectCommandHandler.java#L150

https://github.com/quarkusio/quarkus/blob/main/independent-projects/tools/codestarts/src/main/java/io/quarkus/devtools/codestarts/CodestartProjectInputBuilder.java#L46

@ia3andy
Copy link
Contributor Author

ia3andy commented Oct 14, 2022

@arpitbhardwaj would you give it a try?

@arpitbhardwaj
Copy link

Yeah I will it a try. Currently setting up the environment.

@ia3andy
Copy link
Contributor Author

ia3andy commented Dec 2, 2022

@arpitbhardwaj any update on this?

@gsmet
Copy link
Member

gsmet commented Dec 2, 2022

I'd really like to have it for 2.15 so I think I will drive this puppy home.

@gsmet gsmet self-assigned this Dec 2, 2022
@gsmet
Copy link
Member

gsmet commented Dec 2, 2022

I created a PR here: #29644 .

@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 5, 2022
@maxandersen
Copy link
Contributor

Just to be clear - my suggestion was not to use global system properties. Simply that -D is defining data keys for the command.

What is the difference now between --data and -D ?

@maxandersen
Copy link
Contributor

so I now got to import the code and realize that -D already exist in more generic form. somehow I had in my head for codestarts we had separate dedicated command.

so all good - my only gripe is that it relies on command separated "blob" which limits what you can pass in but we can fix that if usecase arises by adding support for i.e. -QDkey=value or similar.

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 6, 2022
gsmet added a commit to gsmet/quarkus that referenced this issue Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/codestarts area/maven good first issue Good for newcomers kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants