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

[WIP] Shared extension for the Hibernate Search Elasticsearch backend #39416

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Mar 13, 2024

To reduce code duplication.

Opening as draft, because:

Based on ##43943, which must be merged first.

@yrodiere yrodiere marked this pull request as draft March 13, 2024 16:59
@quarkus-bot quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/documentation area/hibernate-search Hibernate Search / Elasticsearch area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Mar 13, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 13, 2024

/cc @gsmet (hibernate-search)

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@yrodiere
Copy link
Member Author

yrodiere commented Oct 9, 2024

Rebased on main. Ran a few tests locally, and it seems to be working fine... we'll need to check the generated documentation for configuration properties though.

@yrodiere yrodiere marked this pull request as ready for review October 9, 2024 14:28
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@yrodiere yrodiere force-pushed the backend-elasticsearch-shared branch from dcbbe40 to 1d3bb33 Compare October 9, 2024 14:58
@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Oct 9, 2024
@quarkus-bot

This comment has been minimized.

@yrodiere yrodiere force-pushed the backend-elasticsearch-shared branch 2 times, most recently from fb9e99c to 5bff5be Compare October 9, 2024 15:11
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

Copy link
Contributor

@marko-bekhta marko-bekhta left a comment

Choose a reason for hiding this comment

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

This is a nice improvement! 😃
There are a couple of failing tests because of the missing pu-config, I'll let you decide how you want to address those 😃. Other than that, it looks good!

@yrodiere
Copy link
Member Author

Thanks for your comments @marko-bekhta , I think I addressed most of them.

Now, the failure to generate documentation is actually more serious than it seemed... I started working on a potential solution, but in essence we're lacking some features in Quarkus docs generation. I'll try to discuss this with Guillaume, see #43808 for details (or don't, and save your time for something more interesting xD).

@yrodiere yrodiere changed the title Shared extension for the Hibernate Search Elasticsearch backend [WIP] Shared extension for the Hibernate Search Elasticsearch backend Oct 10, 2024
@yrodiere yrodiere added the triage/on-ice Frozen until external concerns are resolved label Oct 10, 2024
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@yrodiere
Copy link
Member Author

@gsmet I rebased my branch (this PR) on main and removed any reliance on my work in ##43808. You should be able to test your xref:# link rewriting by rebasing it again on your work.
FWIW, relevant configuration options whose documention include links to the guide:

  • io.quarkus.hibernate.search.backend.elasticsearch.runtime.HibernateSearchBackendElasticsearchRuntimeConfig.LayoutConfig#strategy:
    • quarkus.hibernate-search-orm.elasticsearch.layout.strategy
    • quarkus.hibernate-search-standalone.elasticsearch.layout.strategy
    • Two links: "bean reference", "thisection".
  • io.quarkus.hibernate.search.backend.elasticsearch.runtime.HibernateSearchBackendElasticsearchBuildTimeConfig.AnalysisConfig#configurer:
    • quarkus.hibernate-search-orm.elasticsearch.analysis.configurer
    • quarkus.hibernate-search-standalone.elasticsearch.analysis.configurer
    • Three links: "bean reference", "analysis-configurer", "this section".

Thanks for having a look!

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

gsmet and others added 6 commits October 17, 2024 13:57
In the light of the Spring config metadata, let's get rid of sourceClass
and let's avoid using sourceType for the field/method type as it's
confusing.

Fixes quarkusio#43175
When we attach the config metadata, we need to consider only the
properties of the current extension and not those from potential
internal/common extensions: these will be published for the
internal/common extensions themselves.
The all config guide might reference links from the current guide and we
need to make sure they still work.
@yrodiere yrodiere removed area/core triage/on-ice Frozen until external concerns are resolved area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/dependencies Pull requests that update a dependency file labels Oct 18, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 18, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit e5a7335.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

🎊 PR Preview 5e4a69e has been successfully built and deployed to https://quarkus-pr-main-39416-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 18, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit e5a7335.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.BlockingRawTest.shouldAdd - History

  • Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

⚙️ Maven Tests - JDK 17 Windows

📦 integration-tests/maven

io.quarkus.maven.it.TestMojoIT.testThatTheTestsAreReRunMultiModule - History

  • Condition with Lambda expression in io.quarkus.maven.it.continuoustesting.TestModeContinuousTestingMavenTestUtils was not fulfilled within 3 minutes. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.maven.it.continuoustesting.TestModeContinuousTestingMavenTestUtils was not fulfilled within 3 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.maven.it.continuoustesting.TestModeContinuousTestingMavenTestUtils.waitForNextCompletion(TestModeContinuousTestingMavenTestUtils.java:50)
	at io.quarkus.maven.it.LaunchMojoTestBase.testThatTheTestsAreReRunMultiModule(LaunchMojoTestBase.java:56)

⚙️ Native Tests - HTTP

📦 integration-tests/rest-client-reactive

io.quarkus.it.rest.client.BasicTestIT.shouldCreateClientSpans - History

  • expected: <1> but was: <2> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
	at io.quarkus.it.rest.client.BasicTest.shouldCreateClientSpans(BasicTest.java:216)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:810)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants