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

Fix loading of Red Hat artifacts for catalog #1645

Merged

Conversation

apupier
Copy link
Member

@apupier apupier commented Nov 20, 2024

the test is passing locally for me despite i checked settings.xml to not have the red hat maven repoitory and remove the org.apache/camel folder from my .m2 so maybe another specific part is failing?

relates to #1597

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Please upload report for BASE (main@94ebfb8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../camelcatalog/maven/CamelCatalogVersionLoader.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1645   +/-   ##
=======================================
  Coverage        ?   83.98%           
  Complexity      ?      365           
=======================================
  Files           ?      284           
  Lines           ?     8147           
  Branches        ?     1605           
=======================================
  Hits            ?     6842           
  Misses          ?     1217           
  Partials        ?       88           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@apupier apupier force-pushed the 1597-addRedHatMavenRepositoryAutomatically branch 2 times, most recently from 28a83d4 to 10d5599 Compare November 20, 2024 13:26
@apupier
Copy link
Member Author

apupier commented Nov 20, 2024

I think the reason it works with the test and not from command-line is that camel-yaml-dsl artifact is part of the test dependencies, consequently the schema/camelYamlDsl.json is always picked from this one and not from what is loaded dynamically

@apupier
Copy link
Member Author

apupier commented Nov 20, 2024

io.kaoto.camelcatalog.maven.CamelCatalogVersionLoader.configureRepositories(String) is called too late for Camel Yaml DSL loading. it is called when loading Camel Catalogs.
Potential quick and dirty fix: switch order of loading catalog and camel yaml dsl in io.kaoto.camelcatalog.generator.CatalogGenerator.generate()
better: configuring repositories sooner, or ensuring each of the loader are loading the repositories if they need it

io.kaoto.camelcatalog.maven.CamelCatalogVersionLoader.configureRepositories(String)
is called too late for Camel Yaml DSL loading. it is called when loading
Camel Catalogs.
Quickfix provided: calling the CamelCatalog which is configuring the
repositories at first
not perfect as note all versions are aligned and some others might need
it but should be enough to unblock current situation

note that the provided is not efficient because the camel-yaml-dsl test
dependency is leaking in the other tests, thus even without the fix the
test is passing... I guess it would require to create a separate module
for the test or to ensure a specific version is picked (see also next
paragraph)

Another note: when several catalog are generated at same time, I'm not
sure that we can ensure that it picks the exact correct version of the
camel-yaml-dsl, it might be the first one found, whatever version it is;
To be checked. In practice I think that most of the time there are no
differences.

fixes KaotoIO#1597

Signed-off-by: Aurélien Pupier <[email protected]>
@apupier apupier force-pushed the 1597-addRedHatMavenRepositoryAutomatically branch from 10d5599 to 5c2d1c6 Compare November 20, 2024 14:09
Copy link

sonarcloud bot commented Nov 20, 2024

@apupier
Copy link
Member Author

apupier commented Nov 20, 2024

io.kaoto.camelcatalog.maven.CamelCatalogVersionLoader.configureRepositories(String)
is called too late for Camel Yaml DSL loading. it is called when loading
Camel Catalogs.
Quickfix provided: calling the CamelCatalog which is configuring the
repositories at first
not perfect as note all versions are aligned and some others might need
it but should be enough to unblock current situation

note that the provided is not efficient because the camel-yaml-dsl test
dependency is leaking in the other tests, thus even without the fix the
test is passing... I guess it would require to create a separate module
for the test or to ensure a specific version is picked (see also next
paragraph)

Another note: when several catalog are generated at same time, I'm not
sure that we can ensure that it picks the exact correct version of the
camel-yaml-dsl, it might be the first one found, whatever version it is;
To be checked. In practice I think that most of the time there are no
differences.

@apupier apupier changed the title WIP test reproducer Maven red hat download issue Fix loading of Red Hat artifacts for catalog Nov 20, 2024
@apupier apupier marked this pull request as ready for review November 20, 2024 14:38
@lordrip
Copy link
Member

lordrip commented Nov 20, 2024

When running

./mvnw package; java -jar ./target/catalog-generator-0.0.1-SNAPSHOT.jar -o ./dist/camel-catalog -k 4.6.0 -m 4.4.0.redhat-00045 -n "Default Catalog"

I got
image

@apupier apupier merged commit cd1175d into KaotoIO:main Nov 20, 2024
13 checks passed
@apupier apupier deleted the 1597-addRedHatMavenRepositoryAutomatically branch November 20, 2024 15:20
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.

3 participants