[
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)
