[
https://issues.apache.org/jira/browse/IGNITE-7421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16389669#comment-16389669
]
Vladimir Ozerov commented on IGNITE-7421:
-----------------------------------------
[~kukushal], my comments:
1) By convention we always set {{serialVersionUID}} to {{0L}}.
2) I do not recommend to use {{Objects.hash(...)}} method because it is vararg
and creates byte array underneath every time. It is better to calculate hash
code by hand using standard {{31 * x + y}} algorithm. We should discourage
ourselves from using helpers with hidden effects for simple code parts.
3) {{SslProtocol and SslMode}} - as these things are specific to thin client,
let's move them to thin client package.
4) {{SslProtocol}} - if we have only TLS mode (i.e. SSL is considered totally
deprecated), why would we need this enum at all? Let's remove it along with
relevant properties altogether for the sake of simplicity.
5) {{ClientCacheConfiguration}} is not top-level thing, and hence should be
located inside {{client}} package, not inside
{{org.apache.ignite.configuration}}
6) {{ClientCacheConfiguration}} - collection-based getter/setters should be
symmetircal. I.e. they either accept array (vararg) and return array, or accept
collection and return collection. Arrays is considered better practice at the
moment.
7) {{IgniteClientConfiguration.addrs}} - we should not resolve addresses here,
neither we should expose resolved address with public methods. Instead,
resolution should happen in runtime when connection attempt is performed.
Otherwise your code is not responsive to runtime changes to network
configuration.
8) Taking in count p.7, {{TcpAddressConfigLines}} appears to me a dangerous
abstraction. I would rather convert it to a static stateless method inside
{{IgniteUtils}} which is called in runtime in during connection attempt.
9) {{TcpDiscoveryVmIpFinder}} - I would recommend to revert the changes. It is
considered bad practice to change classes which is unrelated to the ticket. If
refactoring is needed, it should be performed in separate ticket. This approach
minimizes impact of certain changes and simplified review and testing. In this
particular case - {{TcpDiscoveryVmIpFinder}} is central class, very sensitive
to any changes. Any change introduced here may affect the whole project.
Definitely not the right move provided that current ticket is focused on thin
client and should not affect core logic in general.
10) Let's rename {{org.apache.ignite.thinclient}} package to
{{org.apache.ignite.client}}. Word {{thin}} doesn't add much value, because our
non-thin client is already considered deprecated and will be removed soon.
11) {{IgniteUnavailableException}} - I would rename it to
{{ClientConnectionException}}
12) {{IgniteConfigurationException}} - not clear why we need it, provided p.7,
p.8 and p.10. Appears to be a wrong abstraction to me.
13) {{IgniteBinaryMarshaller}} - there should be no static. In principle user
may have different clients connected to different grids. In general, any static
state is discouraged in Ignite. Instead, it is better to have a singleton
marshaller *per-client*.
> Thin client Java API - data grid API
> ------------------------------------
>
> Key: IGNITE-7421
> URL: https://issues.apache.org/jira/browse/IGNITE-7421
> Project: Ignite
> Issue Type: New Feature
> Components: thin client
> Reporter: Alexey Kukushkin
> Assignee: Alexey Kukushkin
> Priority: Major
> Labels: data, java, thin
>
> Implement below Java bindings for the thin client protocol. The client
> configuration must support failover and encryption.
> Cache
> JCache (limited)
> getName(): String
> put(key, val)
> get(key): V
> getAll(keys: Set): Map
> containsKey(key): boolean
> getAndPut(key, val): V
> getAndReplace(key, val): V
> getAndRemove(key): V
> putIfAbsent
> replace(key, val)
> replace(key, oldVal, newVal)
> putAll
> clear
> remove(key)
> remove(key, val)
> removeAll()
> removeAll(keys: Set)
> getConfiguration(clazz): Configuration
> close()
> size(modes: CachePeekMode...)
> query(qry: Query): QueryCursor
> query(qry: SqlFieldsQuery): FieldsQueryCursor<List>
> withKeepBinary(): IgniteCache
> Ignite
> cache(name: String)
> cacheNames(): Collection
> binary(): IgniteBinary
> createCache(name): Cache
> getOrCreateCache(name): Cache
> destroyCache(name)
> Ignition
> startClient(:ClientConfiguration): Ignite
> ClientConfiguration(port, host, binaryConfiguration, sslConfiguration,
> etc...)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
