[
https://issues.apache.org/jira/browse/IGNITE-7421?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16382021#comment-16382021
]
Vladimir Ozerov commented on IGNITE-7421:
-----------------------------------------
[~kukushal], my comments:
1) {{Factory}} - let's use {{javax.cache.configuration.Factory}} instead of our
own interface to avoid duplication; we already do this in many places of our
configuration
2) {{IgniteClientConfiguration}}:
- by convention we put all top-level configuration classes to
{{org.apache.ignite.configuration}}
- we set {{tcpNoDelay}} to {{true}} by default in all other places, let's do
the same here
- let's set {{sslProto}} to {{TLS}} by default explicitly instead of doing this
somewhere deep inside internal classes
- {{credentialsProvider}} factory looks like an overkill for me; plain username
and password strings would be enough, the easier - the better (that is, even
{{Credentials}} class could be removed)
- during JDBC/ODBC development we revealed that setting socket buffer sizes to
OS defaults are typically bad idea because these buffers are too small. Let's
set them to 32Kb by default (we do the same for {{TcpCommunicationSpi}})
- there should be only default constructor; the rest things are set through
settter, this is our product-wide conventions
- addresses should be stored as {{String[]}}, not {{List<InetSocketAddress>}},
relevant setter should accept vararg ({{setAddresses(String...)}}) - this is
proven to be more convenient for users; all further address resolution should
happen outside of configuration class
- configuration should be {{Serializable}} for the sake of user convenience
- I think it is better to expose {{sslProtocol}} as enum rather than as string;
without this user will not know the list of possible values without reading
documentation
- instead of having different setters for host and port, there should be only
one setter {{setAddresses}}. This is how we defined endpoints in other parts of
the system. There should be no defaults - at least one address should always be
provided explicitly
3) {{Credentials}} - not sure we need it at all
4) {{SystemEvent}} - we are going to design common error code glossary soon.
Let's avoid doing this for separate components without seeing the whole
picture. String error message would be enough for now
5) {{ProtocolVersion}} is purely internal thing and should not be exposed to
public API
6) {{SslMode}} - let's rename members to {{REQUIRE*D*}} and {{DISABLE*D*}}
7) I do not think we need {{IgniteClientError}}, {{IgniteProtocolError}},
{{IgniteProtocolVersionMismatch}} and {{IgniteServerError}} for now,
{{IgniteClientException}} should be enough. Moreover, they are not "errors" in
general sense, but "exceptions". Instead, all these cases should be expressed
through a sensible error messages.
8) We need to have common prefixes for all public classes, {{Client}} in this
case; e.g.: {{ClientExcpetion}}, {{ClientConfiguratoin}}, etc.
9) {{IgniteUnavailableException}} - please confirm that exceptions from all
provided addresses are tracked properly here with help of "supressed excpetion"
Java feature
10) {{CacheClientConfiguration}}:
- there should be only default constructor
- class should be {{Serializable}}
- please take all defaults directly from {{CacheConfiguration}}; this way, once
default is changed in CacheConfiguration, it will be applied to
CacheClientConfiguration automcatically, what would help us to maintain API
consistency
- {{setKeyConfiguration}}, {{setQueryEntities}} - let's use varargs here
11) {{IgniteClient.start}} - {{Ignition}} class is a better place for this
method - you can create both a node and a client from a single place. This is
now it is implemented in .NET (and in Hazelcast)
12) {{IgniteClient.nonQuery}} - let's remove this for now. We will design new
SQL API in future and would definitely looks different and located in different
place
13) {{CacheClient.cacheId}} - this should not be exposed to user API, cache ID
is internal thing
> 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)
