MAPRDB-397, MAPRDB-459 #1
Conversation
… language support. Added abstract classes as init commit for ticket.
… language support. Added set of functions for different data types
… language support. Removed functions for driver. Rename DriverManager to ConnectionManager.
… language support. Added part of Value interface and create O*types* files
… language support. MAPRDB-459 Implement ValueImpl class and subclasses like OTime, ODate, etc.. Added part of ValueImpl, OInterval
…, etc.. Added unit test for OInterval, minor changes in other classes and sample code.
…, etc.. Added OTime and ODate implementation
…, etc.. Added ODate implementation.
…, etc.. Added tests for OTime, ODate, OTimestamp. Added new implemented types to ValueImpl.
…for ValueType enum. Added part of Json . Added abstract classes.
adityakishore
left a comment
adityakishore
left a comment
There was a problem hiding this comment.
- Separate the interfaces + public API classes and implementation+tests into individual pull requests.
- Move all the future interfaces (
DocumentReader, etc) into a separate pull request. - Squash multiple commits in a single pull request as one commit.
See comments inline.
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
What's the purpose of this file?
There was a problem hiding this comment.
Accidentally was added
| store = DocumentStore(connection.get_store(store_name="/test_name")) | ||
|
|
||
| # Json string or json map | ||
| json_string = "json string" |
There was a problem hiding this comment.
Please use a valid JSON Document String, something like
{"name": "Joe", "age": 50, "address": {"street": "555 Moon Way", "city": "Gotham"}}
|
|
||
| """Create a connection, get store, find document by id""" | ||
| # create a connection by path:user@password | ||
| connection = Connection(ConnectionManager.get_connection(url="ojai:mapr:user@password")) |
There was a problem hiding this comment.
I am going to update the functional spec with the new URL format, please update the example with a matching format.
There was a problem hiding this comment.
You mean that its must looks like
connection = Connection(ConnectionManager.get_connection(url="hostname:port")) and user@password is not required for connection?
| __metaclass__ = ABCMeta | ||
|
|
||
| @abstractmethod | ||
| def set_id(self, _id): |
There was a problem hiding this comment.
Can this single method accept both binary and string types as "_id" value?
There was a problem hiding this comment.
Yes, here will check which _id type was given
| raise NotImplementedError("Should have implemented this") | ||
|
|
||
| @abstractmethod | ||
| def get_id(self): |
There was a problem hiding this comment.
Would this return Value type? Is there a way to annotate the return type?
There was a problem hiding this comment.
Only in python3.5 and up
https://docs.python.org/3.5/library/typing.html
| @@ -0,0 +1,72 @@ | |||
| class JsonOptions: | |||
There was a problem hiding this comment.
This class is part of the public API and should be separated from implementation.
| raise NotImplementedError("Should have implemented this") | ||
|
|
||
| @abstractmethod | ||
| def check_url(self, url): |
There was a problem hiding this comment.
This is not needed in the abstract class.
There was a problem hiding this comment.
removed check_url from abstract class
| @@ -0,0 +1,13 @@ | |||
|
|
|||
There was a problem hiding this comment.
Not required for initial release.
| raise NotImplementedError("Should have implemented this") | ||
|
|
||
| @abstractmethod | ||
| def as_reader(self): |
There was a problem hiding this comment.
Not required for initial release.
| @@ -0,0 +1,5 @@ | |||
| from abc import ABCMeta, abstractmethod | |||
There was a problem hiding this comment.
Not required for initial release.
No description provided.