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

Yaml configuration not honored. #131

Closed
mike-hampton-float opened this issue Sep 1, 2022 · 21 comments · Fixed by quarkusio/quarkus#28538
Closed

Yaml configuration not honored. #131

mike-hampton-float opened this issue Sep 1, 2022 · 21 comments · Fixed by quarkusio/quarkus#28538
Labels
bug Something isn't working

Comments

@mike-hampton-float
Copy link

mike-hampton-float commented Sep 1, 2022

My configuration is in a yaml file.

My openapi file is here crm-svc/service/src/main/openapi/sellerdeal.yaml

My application.yaml file has

quarkus:
  openapi-generator:
    codegen:
      spec:
        sellerdeal_yaml:
          base-package:
            com.floatfin.sellerdeal.openapi

Expected

The generated interfaces have the package:

com.floatfin.sellerdeal.openapi;

Actual

The generated interfaces have the package:

package org.openapi.quarkus.sellerdeal_yaml.api;
@ricardozanini ricardozanini added the bug Something isn't working label Sep 2, 2022
@ricardozanini
Copy link
Member

Hi @mike-hampton-float! Thanks for reporting it. I'll take a look.

@vladaman
Copy link

vladaman commented Sep 6, 2022

I have same issue. There is different behavior depending on how you run the generator.

  1. mvn clean quarkus:generate-code doesn't honor configuration option and generates code with org.openapi.quarkus package
  2. mvn clean compile generates code properly

@ricardozanini
Copy link
Member

hmm, it might be a bug in Quarkus, then. Thanks for sharing @vladaman. I'll take a look later this week.

@ricardozanini
Copy link
Member

By the way, @gastaldi @geoand, do you guys know or have you seen anything like this before?

@gastaldi
Copy link
Member

gastaldi commented Sep 6, 2022

No idea, but is this reproducible using application.properties too?

@ricardozanini
Copy link
Member

No, using application.properties works. :(

@gastaldi
Copy link
Member

gastaldi commented Sep 8, 2022

It may indicate that the YAML ConfigSource is not found during quarkus:generate-code. WDYT @aloubyansky ?

@hbelmiro
Copy link
Contributor

hbelmiro commented Oct 7, 2022

@gastaldi @aloubyansky

When the implementation of CodeGenProvider runs, the quarkus-config-yaml extension has not yet been loaded.

Would that be the expected behavior or a bug?

@aloubyansky
Copy link
Member

It's a bug, imo. The yaml config source has been loaded by that time but the config instance passed to the code generator doesn't include it.
@radcortez this looks like a consequence of quarkusio/quarkus#21887 and quarkusio/quarkus@ff95ddf that resulted in https://github.com/quarkusio/quarkus/blob/main/core/deployment/src/main/java/io/quarkus/deployment/CodeGenerator.java#L156-L162 Could we do better here?

@aloubyansky
Copy link
Member

It seems like we don't want to load config sources from the app module itself but do want to support those found in the dependencies (whether they belong to the same multi module project or not). We should be able to achieve that with classpath filtering.

@aloubyansky
Copy link
Member

aloubyansky commented Oct 7, 2022

This is the idea aloubyansky/quarkus@309480d @radcortez am I missing something?

@radcortez
Copy link

Yes, commented on the commit :)

@ricardozanini
Copy link
Member

Thanks guys!

@hbelmiro
Copy link
Contributor

Thanks for the quick fix @aloubyansky and @radcortez.
However, this extension needs exactly the config that is in the root app, which is not contemplated in the fix.
I think there's a good reason for not existing a YAML alternative for ApplicationPropertiesConfigSourceLoader, but I'll ask anyway. Wouldn't it be possible to create a YamlPropertiesConfigSourceLoader to cover this use case?

@aloubyansky
Copy link
Member

@hbelmiro do you have a config or a config provider impl? Unless it's the config provider, it should work. Is there a chance you could give it a try? You'd need to build the main branch of Quarkus. Let me know if you need help/instructions to do that.

@ricardozanini ricardozanini reopened this Oct 14, 2022
@hbelmiro
Copy link
Contributor

@aloubyansky I tried and it didn't work. I might have done something wrong though.

The scenario is the following:
The user creates a Quarkus app with a src/main/resources/application.yaml and the quarkus-openapi-generator dependency (this extension). During a mvn compile, this extension needs to read the config from application.yaml to perform its job.

Should it work?

@aloubyansky
Copy link
Member

Yes. Do you have a reproducer ready to try?

@aloubyansky
Copy link
Member

Do you have quarkus-config-yaml among the project's dependencies?

@hbelmiro
Copy link
Contributor

I definitely did something wrong. It's not using Quarkus 999-SNAPSHOT.
I'll try again and if it doesn't work, I'll provide a reproducer.

Do you have quarkus-config-yaml among the project's dependencies?

Yes.

@hbelmiro
Copy link
Contributor

@aloubyansky It worked. Sorry for wasting your time. Thanks for the help :)

@ricardozanini we can close this issue.

@aloubyansky
Copy link
Member

Thanks for confirming @hbelmiro!

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
7 participants