Infinispan client docs #786

wburns · 2019-02-06T21:01:43Z

This contains 2 issues:

Add Shamrock Infinispan Client documentation
Add Infinispan Client to the extensions list

The second one shouldn't be needed if #663 is integrated first.

Sanne

Great progress, thanks!

I've made several remarks that could be addressed, or not. Up to you.

The one critical issue which should be addressed before merging is the missing "<" in the last XML block.

Some of the other ones make me think we need to improve usability further. Feel free to not address them but open tickets to track them as future enhancements?

Sanne · 2019-02-06T21:06:49Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+= {project-name} - Infinispan Client
+
+Infinispan is an in memory data grid that allows running in a server outside of application processes. This extension
+provides functionality to allow the client that can connect to said server when running in protean.


best to try sticking to {project-name} rather than protean .. no biggie though, I guess we'll have to run some search and replace anyway ;)

Sure, I can fix that.

Sanne · 2019-02-06T21:10:11Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+will be automatically picked up during the static initialization time from the `META-INF`
+directory and parsed as it would normally.
+
+You can also define some properties in the `microprofile-config.properties` file that can be


I think I'd switch this around: present just a single way to do things, which is to use the Protean configuration.

at the end you can add a note for advanced users pointing out that using hotrod-client.properties is also an option.

Try to keep the language short and to the point? "If you are familiar or noticed in the above client server guide," can all be removed.

Yeah, I had originally talked with @tristantarrant and @emmanuelbernard about this and they wanted just minimal things in the microprofile config file. I am fine moving these over if we want, but we should have a separate issue for that.

Sure I can remove the extra wording.

+1 we must explain how you do the main things the easy way. And then explain the more and more complex things. If someone can do the job with out having to read the fact that they can use hot rod.propetries then we win.

Sanne · 2019-02-06T21:13:21Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+
+By default the client will only support keys and values of the type byte[]. This can be configured
+through the `hotrod-client.properties` file mentioned above through the `infinispan.client.hotrod.marshaller`
+property. You can either supply your own marshaller implementation or use the `ProtoStreamMarshaller` as described


Could we avoid the need for using the hotrod-client.properties ?

I'd welcome the option to either set this directly in the Protean config, or somehow auto-discover such additional marshallers. Remember we have Jandex so that might be possible ;)

The mantra is to try hard to make it all work with the single config file, and if possible at all to not need config. UX to 11 ;)

I agree, it would be nice - however I think we need to tackle this elsewhere.

I am aware of Jandex :) Although it seems like I should probably refrain from talking about a custom marshaller.

The more I am thinking about this with the comments you guys mentioned, I am thinking I should probably add infinispan-remote-query-client as a dependency to the shamrock infinispan client module and always have the ProtoStreamMarshaller as default.

BTW making microprofile coding having priority over hot rod property file, is that the right thing? I think we do the opposite in Hibernate ORM and even throw exception when there is conflict. @wburns @Sanne thoughts ?

By default the client will only support keys and values of the type byte[].

Soooo, I am scared. I am a web app developer and I don’t understand that. What does that mean? What’s my limitations. What does it means for me as a code usage?

The other thing that scares me is that what you are saying is that by défaut, it sucks and is unusable ? If that’s true, then we don’t make it the default. I want défaut to be the most useful one.

I want loads more concrete code usage examples so I can see what value you provide to me. I don’t know Infinispan. I want lists of use cases and examples

The philosophy of these docs is not to be descriptive but to explain how someone do things.

PS I’m in meeting so apologies if things sound harsh or are just quick directions vs a detailed feedback

+1 right the "custom/more powerful/framework specific" file should have priority.

In Hibernate, you either use one or the other - strict exclusivity.

We'll track this as a separate improvement though, the docs to be merged soon & then we iterate on usability.

Sounds good re: config priority.

Also @emmanuelbernard talking over this more, I am planning on making proto stream the default marshaller and add the optional dependency as required. This will allow all the primitive wrappers, String, byte[], Date, Instant to be serialized with no config. And the user can add their own custom classes themselves.

Sanne · 2019-02-06T21:14:00Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+property. You can either supply your own marshaller implementation or use the `ProtoStreamMarshaller` as described
+in the user guide linked above.
+
+The recommended approach is to use `ProtoStreamMarshaller` as this provides support for String,


So.. should we register this one by default?

Actually didn't see this until after I wrote my previous comment, but that is what I am leaning towards now.

Sanne · 2019-02-06T21:14:55Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+</dependency>
+----
+
+Note that you must exclude `jboss-logging` as this module is currently not supported with shamrock.


s/shamrock/{project-name}/

Sanne · 2019-02-06T21:25:21Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+== Security
+
+The Infinispan Client comes already packed with features to allow for security. However to configure
+these you may need to slightly tweak them to work within Shamrock.


is something missing here? I have no clue of what I would need doing :)

Weird... I must have accidentally deleted this section.

Sanne · 2019-02-06T21:26:27Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+and/or keystore. This is further detailed at
+http://infinispan.org/docs/dev/user_guide/user_guide.html#hr_encryption.
+
+The reason that Shamrock is different is that the Native does not come with security


s/the Native/ SubstrateVM ?

👍 Wasn't sure what naming we wanted here, disregarding the typo in grammar.

Sanne · 2019-02-06T21:27:36Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+           </goals>
+           <configuration>
+               <enableHttpUrlHandler>true</enableHttpUrlHandler>
+               <!-- next two are to enable security - If not needed it is recommended not to enable these-->


makes me think: should we autoamtically enable enableJni when enableAllSecurityServices is set? Or at least validate that the dependent is set?

I was wondering the same thing, I am not sure who normally handles that.

Sanne · 2019-02-06T21:28:40Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+           </configuration>
+       </execution>
+   </executions>
+/plugin>


There's a missing "<" before "/plugin>"

Sanne · 2019-02-06T21:29:34Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+== Additional Features
+
+The Infinispan Client has additional features that were not mentioned here. This means this
+feature was not tested in a Shamrock environment and they may or may not work. Please let us


nice touch!

galderz · 2019-02-07T10:34:14Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+            "}");
+   }
+----
+User Marshaller::


I think the use of protostream could be improved. As it stands, the definition of a type is duplicated in 3 places: the java source file of the type, the proto file and the marshaller. Any changes to any requires modifying all 3. Instead, I find a better approach to annotate the type and let the proto file and marshaller be generated. I did this last year in the streaming data demos I did, e.g. see this train station and related types. Some of the fields are annotated with indexing stuff, but you can skip those for this example. You should try this out yourself.

They don't work with SubstrateVM as I was trying to mention at https://github.com/jbossas/protean-shamrock/pull/786/files#diff-656c2d72aa36b0cb773025ff8b46304aR191

This requires more changes from protostream. I was originally going to do something myself with Jandex, but Adrian wanted to fix it directly in protostream.

When can we have it from adrien in a release and integrated in Protean?
If that’s more than 3 weeks, then we do it in the extension first and fix upstream later. We have applied this strategy in a few places like RESTEasy and Hibernate ORM

My guess is that 3 weeks is a bit too tight for Adrian to get the proto stream changes and add the changes to this extension. The Jandex/Gizmo version should be doable in < 2 weeks I would think. It is more learning how to use them

galderz · 2019-02-07T11:03:38Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+NOTE: Annotation based proto stream marshalling is not yet supported in
+the shamrock infinispan client.
+
+=== Providing your own Marshaller


Not sure I'd advertise this. Although there are some users for it and some domain-specific implementations can really do fast stuff by adding their own marshaller, I find they're a niche. Proto based marshalling is the best for end users since it paths the way to indexing+querying.

Yeah I originally had this section larger, but pared it down. I am fine removing from the documentation.

I will take out this section when I change over to have the default proto stream marshaller.

galderz · 2019-02-07T13:25:53Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+
+== Querying
+
+The Infinispan client supports both indexed and non indexed querying as long as the 


Also: consider to not allow overriding the marshaller? What are the implications?

I've mentioned about this topic above. Overriding the marshaller is not something I'd expose to start with.

galderz · 2019-02-07T13:30:42Z

docs/src/main/asciidoc/infinispan-client-guide.adoc

+=== Authentication
+
+This chart illustrates what mechanisms have been verified to be working properly with
+the Shamrock Infinispan Client extension.


wburns · 2019-02-07T16:17:57Z

I have changed the comments I could and created the following issues to handle the bigger issues with setup.

#798
#800

Sanne · 2019-02-07T18:13:56Z

Added #802

Sanne · 2019-02-07T18:33:18Z

Added also: #804

Sanne · 2019-02-07T18:36:07Z

aand #805 :)

Fixes quarkusio#786

wburns added 2 commits February 6, 2019 15:59

Added Infinispan Client User Guide

5aebe2a

Added Infinispan to extensions list

424dcb4

wburns requested review from Sanne and tristantarrant February 6, 2019 21:01

Sanne self-assigned this Feb 6, 2019

Sanne requested changes Feb 6, 2019

View reviewed changes

galderz requested changes Feb 7, 2019

View reviewed changes

Adding rework changes

5c944dc

wburns mentioned this pull request Feb 7, 2019

Infinispan Client should use ProtoStreamMarshaller as default #798

Closed

Sanne approved these changes Feb 7, 2019

View reviewed changes

Sanne merged commit 37da5c6 into quarkusio:master Feb 7, 2019

Sanne deleted the infinispan-client-docs branch February 7, 2019 18:36

gsmet added this to the 0.9.0 milestone Feb 11, 2019

maxandersen pushed a commit to maxandersen/quarkus that referenced this pull request Nov 5, 2022

fix: Not duplicating or misquoting java options anymore (quarkusio#787)

9423880

Fixes quarkusio#786


		== Querying

		The Infinispan client supports both indexed and non indexed querying as long as the

Infinispan client docs #786

Infinispan client docs #786

Conversation

wburns commented Feb 6, 2019

Sanne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmanuelbernard Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galderz Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmanuelbernard Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wburns commented Feb 7, 2019

Sanne commented Feb 7, 2019

Sanne commented Feb 7, 2019

Sanne commented Feb 7, 2019

emmanuelbernard Feb 7, 2019 •

edited

Loading

galderz Feb 7, 2019 •

edited

Loading

emmanuelbernard Feb 7, 2019 •

edited

Loading