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

Clearing code samples from internal API's and asserts. #306

Open
sancar opened this issue Feb 28, 2019 · 1 comment
Open

Clearing code samples from internal API's and asserts. #306

sancar opened this issue Feb 28, 2019 · 1 comment

Comments

@sancar
Copy link
Contributor

sancar commented Feb 28, 2019

I don't know if it is discussed before. Our code samples use internal API, which caused compile problem in this release when changed. Fixed here https://github.com/hazelcast/hazelcast-code-samples/pull/303/files
The reason of usage seems to demonstrate some features are working. And it resembles our unit tests.
I think the code sample does not need that much verification. Since it is to show how hazelcast should be used, it is giving wrong advice.

Heaviest usage is around simulating the split brain. That requires access to node. I think this should not be part of the code samples. Configuring split brain on code samples make sense. But simulating the behavior I think is part of the unit tests.
https://github.com/hazelcast/hazelcast-code-samples/blob/master/jcache/src/main/java/com/hazelcast/examples/splitbrain/AbstractCacheSplitBrainSample.java#L105

I see generateKeyOwnedBy usage. I don't think we are advising this to our users. Make sense for tests but users should use affinity features instead of generating the key for a specific partition. And interestingly, this also accesses to node.

I think we also need rules for using assertions. We have the following methods on Common Utils. My gut feeling says that code samples should not do assert.
.assertClusterSizeEventually;
.assertEquals;
.assertOpenEventually;

I am keen to do the following
System.out.println(cache1.get("key1"));//prints 'value'
instead of
assertEquals("value",);

Lastly, I see abstract class usages too much. Especially around SplitBrainSamples. I think code samples should be one class and simple. Using abstractions to demonstrate simple usages will scare of the users. This again seems to be caused by simulating the split brain. When we remove that part, abstractions will go away too.

@sancar sancar added this to the 3.12 milestone Feb 28, 2019
@sancar sancar self-assigned this Feb 28, 2019
@sancar sancar removed their assignment Nov 12, 2019
@mmedenjak mmedenjak modified the milestones: 3.12, 4.1 Mar 17, 2020
@mmedenjak
Copy link
Contributor

Moving to 4.1

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

No branches or pull requests

3 participants