[jira] [Comment Edited] (IGNITE-8014) Node.js client: basic/minimal version

["Vladimir Ozerov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-8014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16411186#comment-16411186 ] 

Vladimir Ozerov edited comment on IGNITE-8014 at 3/23/18 10:41 AM:
-------------------------------------------------------------------

[~alexey.kosenchuk], in general code looks good. I have several comments regarding public API behavior:
1) {{CacheClient}} - what is the purpose of  {{setKeyType}} and {{setValueType}} methods from user perspective? In other languages we do not have such restriction and cache can store entries of different types (though, we tend to think of it as an antipattern). I am not an expert in NodeJS, may be this is a consequence of NodeJS type system?
2) {{Errors}} - typically we try to have as least different error types as possible. In this specific case:
- {{IllegalArgumentError}} - looks good
- {{OperationError}} - looks good
- {{TypeCastError}}, {{UnsupportedTypeError}} - looks good
- {{IllegalStateError}}, {{LostConnectionError}} - appears to be the same thing - client is not connected; I would merge it into a single entity
- {{InternalError}} - I doubt user has any chance to react to this error in any way except of logging or rethrowing; I would remove it and use base IgniteClientError instead


was (Author: vozerov):
[~alexey.kosenchuk], in general code looks good. I have several comments regarding public API behavior:
1) {{CacheClient}} - what is the purpose of  {{setKeyType}} and {{setValueType}} methods from user perspective? In other languages we do not have such restriction and cache can store entries of different types (though, we tend to think of it as an antipattern). I am not an expert in NodeJS, may be this is a consequence of NodeJS type system?
2) {{Errors}} - typically we try to have as least different error types as possible. In this specific case:
- {{IllegalArgumentError}} - looks good
- {{TypeCastError}}, {{UnsupportedTypeError}} - looks good
- {{IllegalStateError}}, {{LostConnectionError}} - appears to be the same thing - client is not connected; I would merge it into a single entity
- {{InternalError}} - I doubt user has any chance to react to this error in any way except of logging or rethrowing; I would remove it and use base IgniteClientError instead

> Node.js client: basic/minimal version
> -------------------------------------
>
>                 Key: IGNITE-8014
>                 URL: https://issues.apache.org/jira/browse/IGNITE-8014
>             Project: Ignite
>          Issue Type: Sub-task
>          Components: thin client
>            Reporter: Alexey Kosenchuk
>            Assignee: Alexey Kosenchuk
>            Priority: Major
>
> Develop the first basic/minimal version - for initial review/feedback.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)