-
Notifications
You must be signed in to change notification settings - Fork 1
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 #9 to add config to register classes #17
Fix #9 to add config to register classes #17
Conversation
51b16d9
to
1f79b9a
Compare
runtime/src/main/java/io/quarkiverse/fury/FuryBuildTimeConfig.java
Outdated
Show resolved
Hide resolved
|
||
|
||
ifdef::add-copy-button-to-env-var[] | ||
Environment variable: env_var_with_copy_button:+++QUARKUS_FURY_REGISTER_CLASSES__REGISTER_CLASS_NAME__CLASS_ID+++[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment variable: env_var_with_copy_button:+++QUARKUS_FURY_REGISTER_CLASSES__REGISTER_CLASS_NAME__CLASS_ID+++[] | |
Environment variable: env_var_with_copy_button:+++QUARKUS_FURY_REGISTER_CLASSES_REGISTER_CLASS_NAME_CLASS_ID+++[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, This is generated by quarkus-maven-plugin
. I think the double _
means that the users need to replace it with the real class name. So the environment looks like QUARKUS_FURY_REGISTER_CLASSES__io.quarkiverse.fury.it.Foo__CLASS_ID
.
@gastaldi is it right? I assume that this is similar with the logger configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, @zhfeng is right
docs/modules/ROOT/pages/includes/quarkus-fury_quarkus.fury.adoc
Outdated
Show resolved
Hide resolved
docs/modules/ROOT/pages/includes/quarkus-fury_quarkus.fury.adoc
Outdated
Show resolved
Hide resolved
docs/modules/ROOT/pages/includes/quarkus-fury_quarkus.fury.adoc
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,2 @@ | |||
quarkus.fury.register-class-names=io.quarkiverse.fury.it.Struct | |||
quarkus.fury.register-classes."io.quarkiverse.fury.it.Foo".class-id=300 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share en exmaple how to configure multiple class with class id and serializer in application.properties
?
Does it look like following code? If so, I perfer name configuration key as register-class
instead of register-classes
quarkus.fury.register-classes."io.quarkiverse.fury.it.Foo".class-id=300
quarkus.fury.register-classes."io.quarkiverse.fury.it.Foo".serialize=io.quarkiverse.fury.it.FooSerializer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use register_classes
instead of register-classes
, will quarkus still parse config correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it look like following code? If so, I perfer name configuration key as register-class instead of register-classes
Yeah, it is. I can change it to register-class
.
If we use register_classes instead of register-classes, will quarkus still parse config correctly?
No, it will not. I think Quarkus uses microprofile
config spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, let's change to register-class
. And it would be great if we can add a serializer config here for test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.