Skip to content

Conversation

@JiriOndrusek
Copy link
Contributor

@JiriOndrusek JiriOndrusek commented Jul 15, 2021

fixes #2755
fixes #2255

In his change I propose registration for serialization of several basic classes. But because this is not required by the most of the extensions, I suppose that creating a buildItem and based registration on it, would be better. @ppalaga @jamesnetherton WDYT?
Another problem is BigInteger, which doesn't work - I have to find why.

@jamesnetherton
Copy link
Contributor

I suppose that creating a buildItem and based registration on it

Yeah, that's how I would have implemented it. Otherwise every extension that wants serialization support has to duplicate the required steps.

I guess the registration of the basic Java types ideally belongs in Quarkus itself.

@JiriOndrusek
Copy link
Contributor Author

JiriOndrusek commented Jul 15, 2021

I agree that basic stuff would be in quarkus - in the future. (I plan to create a PR there)
But some stuff like org.apache.camel.support.DefaultExchangeHolder would stay here.

Thanks for your response!

@JiriOndrusek JiriOndrusek force-pushed the 2755-serialization-for-nitrite-and-sql branch from 9ec6d48 to 6b4ba97 Compare July 15, 2021 07:54
@lburgazzoli
Copy link
Contributor

lburgazzoli commented Jul 15, 2021

oh this is really nice.

btw we should really add a task to revisit DefaultExchangeHolder as it has been designed when serialization was cool but now it should be better to use json/avro/protobof

@JiriOndrusek JiriOndrusek changed the title Use of serialization feature of Quakus (fro Sql and Minio) Use of serialization feature of Quakus (includes Sql and Nitrite) Jul 15, 2021
@JiriOndrusek JiriOndrusek force-pushed the 2755-serialization-for-nitrite-and-sql branch 8 times, most recently from 923bc7a to 4d6de8c Compare July 15, 2021 13:05
@JiriOndrusek JiriOndrusek marked this pull request as ready for review July 15, 2021 13:06
@zbendhiba
Copy link
Contributor

btw we should really add a task to revisit DefaultExchangeHolder as it has been designed when serialization was cool but now it should be better to use json/avro/protobof

+1

@jamesnetherton
Copy link
Contributor

@JiriOndrusek please rebase on main as Quarkus has been bumped to 2.1.0.CR1.

@JiriOndrusek
Copy link
Contributor Author

I'll rebase
and here is an issue about DefaultExchangeHolder - https://issues.apache.org/jira/browse/CAMEL-16805

@JiriOndrusek JiriOndrusek changed the base branch from quarkus-main to main July 19, 2021 10:33
@JiriOndrusek JiriOndrusek force-pushed the 2755-serialization-for-nitrite-and-sql branch 2 times, most recently from 69cf846 to 4f1e7a8 Compare July 19, 2021 11:48
Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Nice work, thanks @JiriOndrusek! Some comments and questions inline.

@jamesnetherton
Copy link
Contributor

Is there any work remaining on this one? It'd be good to get it merged ASAP as there's a few other things that depend on it.

@JiriOndrusek
Copy link
Contributor Author

Please leave me about an hour to add changes pointed by @ppalaga

@JiriOndrusek JiriOndrusek force-pushed the 2755-serialization-for-nitrite-and-sql branch 2 times, most recently from 0b9ce1b to 4f05564 Compare July 20, 2021 07:47
@JiriOndrusek
Copy link
Contributor Author

@jamesnetherton All items are fixed. There is no more remaining work on this PR.

@JiriOndrusek JiriOndrusek force-pushed the 2755-serialization-for-nitrite-and-sql branch from 4f05564 to 2151e0f Compare July 20, 2021 09:29
@ppalaga ppalaga merged commit 358d26a into apache:main Jul 20, 2021
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.

Refactor registration of serialization once support is present in Quarkus Nitrite - follow up - simplify serialization configuration

5 participants