[DHT] Queries: remote values filtering #79

Merged
merged 4 commits into from Jul 11, 2016

Conversation

Projects
None yet
3 participants
@sim590
Contributor

sim590 commented Jun 23, 2016

Since #70 had been merged, but then also secretely reverted, I'm referring to this PR. This is essentially containing the same code, but also addressing the matter discuessed in this comment properly.

This PR now includes:

  • DhtRunner::query API method;
  • QueryCallback callback used in DhtRunner::query signature;
  • Proper Query::Query(std::string) initialization with both Select and Where functionnal structures;
  • dhtnode is updated accordingly and introduces the command q which will let you start a query;

The following should be added:

  • python bindings for all the features listed above;

I'm not sure if I'm missing something from the lists above. @aberaud: feel free to comment.

@sim590

This comment has been minimized.

Show comment
Hide comment
@sim590

sim590 Jun 27, 2016

Contributor

I have worked on getting python bindings here. It's giving me truble with binding with C enum... The crash error I get seems to indicate, this is a cython bug.

Error compiling Cython file:
------------------------------------------------------------
...
        self._index.reset(new cpp.FieldValueIndex())
    def __str__(self):
        return self._index.get().toString().decode()
    property id:
        def __get__(self):
            return self._index.get().index[cpp.Value.Field.Id].getInt()
                                                         ^
------------------------------------------------------------

/home/simon/prog/opendht/python/opendht.pyx:186:58: Compiler crash in AnalyseExpressionsTransform

ModuleNode.body = StatListNode(opendht.pyx:30:0)
StatListNode.stats[12] = StatListNode(opendht.pyx:178:5)
StatListNode.stats[0] = CClassDefNode(opendht.pyx:178:5,
    as_name = 'FieldValueIndex',
    class_name = 'FieldValueIndex',
    module_name = '',
    visibility = 'private')
CClassDefNode.body = StatListNode(opendht.pyx:179:4)
StatListNode.stats[2] = PropertyNode(opendht.pyx:184:4,
    name = 'id')
PropertyNode.body = StatListNode(opendht.pyx:185:8)
StatListNode.stats[0] = DefNode(opendht.pyx:185:8,
    modifiers = [...]/0,
    name = '__get__',
    num_required_args = 1,
    py_wrapper_required = True,
    reqd_kw_flags_cname = '0',
    used = True)
File 'Nodes.py', line 429, in analyse_expressions: StatListNode(opendht.pyx:186:12,
    is_terminator = True)
File 'Nodes.py', line 5549, in analyse_expressions: ReturnStatNode(opendht.pyx:186:12,
    is_terminator = True)
File 'ExprNodes.py', line 5010, in analyse_types: SimpleCallNode(opendht.pyx:186:69,
    analysed = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6227, in analyse_types: AttributeNode(opendht.pyx:186:62,
    attribute = 'getInt',
    initialized_check = True,
    is_attribute = 1,
    is_called = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6339, in analyse_as_ordinary_attribute_node: AttributeNode(opendht.pyx:186:62,
    attribute = 'getInt',
    initialized_check = True,
    is_attribute = 1,
    is_called = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 3314, in analyse_types: IndexNode(opendht.pyx:186:42,
    is_subscript = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 3363, in analyse_base_and_index_types: IndexNode(opendht.pyx:186:42,
    is_subscript = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6225, in analyse_types: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6291, in analyse_as_type_attribute: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6334, in as_name_node: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1902, in analyse_rvalue_entry: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1935, in analyse_entry: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1949, in check_identifier_kind: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)

Compiler crash traceback from this point on:
  File "/usr/lib/python3.5/site-packages/Cython/Compiler/ExprNodes.py", line 1949, in check_identifier_kind
    if entry.is_type and entry.type.is_extension_type:
AttributeError: 'NoneType' object has no attribute 'is_type'
missing cimport in module 'opendht_cpp': /home/simon/prog/opendht/python/opendht.pyx
Compiling /home/simon/prog/opendht/python/opendht.pyx because it changed.
[1/1] Cythonizing /home/simon/prog/opendht/python/opendht.pyx
Traceback (most recent call last):
  File "setup.py", line 56, in <module>
    library_dirs = ['/home/simon/prog/opendht/build/python', '/home/simon/prog/opendht/build']
  File "/usr/lib/python3.5/site-packages/Cython/Build/Dependencies.py", line 912, in cythonize
    cythonize_one(*args)
  File "/usr/lib/python3.5/site-packages/Cython/Build/Dependencies.py", line 1034, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: /home/simon/prog/opendht/python/opendht.pyx
make[2]: *** [python/CMakeFiles/python.dir/build.make:59: python/CMakeFiles/python] Error 1
make[1]: *** [CMakeFiles/Makefile2:293: python/CMakeFiles/python.dir/all] Error 2
make: *** [Makefile:128: all] Error 2

I'll dig a little bit more on this later.

@aberaud Now, I think that the queries branch could be merged without python bindings.

Contributor

sim590 commented Jun 27, 2016

I have worked on getting python bindings here. It's giving me truble with binding with C enum... The crash error I get seems to indicate, this is a cython bug.

Error compiling Cython file:
------------------------------------------------------------
...
        self._index.reset(new cpp.FieldValueIndex())
    def __str__(self):
        return self._index.get().toString().decode()
    property id:
        def __get__(self):
            return self._index.get().index[cpp.Value.Field.Id].getInt()
                                                         ^
------------------------------------------------------------

/home/simon/prog/opendht/python/opendht.pyx:186:58: Compiler crash in AnalyseExpressionsTransform

ModuleNode.body = StatListNode(opendht.pyx:30:0)
StatListNode.stats[12] = StatListNode(opendht.pyx:178:5)
StatListNode.stats[0] = CClassDefNode(opendht.pyx:178:5,
    as_name = 'FieldValueIndex',
    class_name = 'FieldValueIndex',
    module_name = '',
    visibility = 'private')
CClassDefNode.body = StatListNode(opendht.pyx:179:4)
StatListNode.stats[2] = PropertyNode(opendht.pyx:184:4,
    name = 'id')
PropertyNode.body = StatListNode(opendht.pyx:185:8)
StatListNode.stats[0] = DefNode(opendht.pyx:185:8,
    modifiers = [...]/0,
    name = '__get__',
    num_required_args = 1,
    py_wrapper_required = True,
    reqd_kw_flags_cname = '0',
    used = True)
File 'Nodes.py', line 429, in analyse_expressions: StatListNode(opendht.pyx:186:12,
    is_terminator = True)
File 'Nodes.py', line 5549, in analyse_expressions: ReturnStatNode(opendht.pyx:186:12,
    is_terminator = True)
File 'ExprNodes.py', line 5010, in analyse_types: SimpleCallNode(opendht.pyx:186:69,
    analysed = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6227, in analyse_types: AttributeNode(opendht.pyx:186:62,
    attribute = 'getInt',
    initialized_check = True,
    is_attribute = 1,
    is_called = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6339, in analyse_as_ordinary_attribute_node: AttributeNode(opendht.pyx:186:62,
    attribute = 'getInt',
    initialized_check = True,
    is_attribute = 1,
    is_called = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 3314, in analyse_types: IndexNode(opendht.pyx:186:42,
    is_subscript = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 3363, in analyse_base_and_index_types: IndexNode(opendht.pyx:186:42,
    is_subscript = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6225, in analyse_types: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6291, in analyse_as_type_attribute: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 6334, in as_name_node: AttributeNode(opendht.pyx:186:58,
    attribute = 'Id',
    initialized_check = True,
    is_attribute = 1,
    needs_none_check = True,
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1902, in analyse_rvalue_entry: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1935, in analyse_entry: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)
File 'ExprNodes.py', line 1949, in check_identifier_kind: NameNode(opendht.pyx:186:58,
    cf_maybe_null = True,
    is_name = True,
    name = 'Id',
    result_is_used = True,
    use_managed_ref = True)

Compiler crash traceback from this point on:
  File "/usr/lib/python3.5/site-packages/Cython/Compiler/ExprNodes.py", line 1949, in check_identifier_kind
    if entry.is_type and entry.type.is_extension_type:
AttributeError: 'NoneType' object has no attribute 'is_type'
missing cimport in module 'opendht_cpp': /home/simon/prog/opendht/python/opendht.pyx
Compiling /home/simon/prog/opendht/python/opendht.pyx because it changed.
[1/1] Cythonizing /home/simon/prog/opendht/python/opendht.pyx
Traceback (most recent call last):
  File "setup.py", line 56, in <module>
    library_dirs = ['/home/simon/prog/opendht/build/python', '/home/simon/prog/opendht/build']
  File "/usr/lib/python3.5/site-packages/Cython/Build/Dependencies.py", line 912, in cythonize
    cythonize_one(*args)
  File "/usr/lib/python3.5/site-packages/Cython/Build/Dependencies.py", line 1034, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: /home/simon/prog/opendht/python/opendht.pyx
make[2]: *** [python/CMakeFiles/python.dir/build.make:59: python/CMakeFiles/python] Error 1
make[1]: *** [CMakeFiles/Makefile2:293: python/CMakeFiles/python.dir/all] Error 2
make: *** [Makefile:128: all] Error 2

I'll dig a little bit more on this later.

@aberaud Now, I think that the queries branch could be merged without python bindings.

@kaldoran

This comment has been minimized.

Show comment
Hide comment
@kaldoran

kaldoran Jun 28, 2016

Collaborator

If the problem is really Cython, then this patch should be merge without python enable.
But we need to be sure that its a Cython problem, if so, we should also realated the bug to Cython.

Ps : There is a lot of commit, isn't some that could be squash into one ?

Collaborator

kaldoran commented Jun 28, 2016

If the problem is really Cython, then this patch should be merge without python enable.
But we need to be sure that its a Cython problem, if so, we should also realated the bug to Cython.

Ps : There is a lot of commit, isn't some that could be squash into one ?

@sim590

This comment has been minimized.

Show comment
Hide comment
@sim590

sim590 Jun 28, 2016

Contributor

@kaldoran. You're right. I will squash everything in seperate relevant commits. I have not included cython into this PR, so don't bother the cython error.

Contributor

sim590 commented Jun 28, 2016

@kaldoran. You're right. I will squash everything in seperate relevant commits. I have not included cython into this PR, so don't bother the cython error.

@sim590

This comment has been minimized.

Show comment
Hide comment
@sim590

sim590 Jun 28, 2016

Contributor

@kaldoran: since you seem to be available for the review, I've also assigned you to this PR.

Contributor

sim590 commented Jun 28, 2016

@kaldoran: since you seem to be available for the review, I've also assigned you to this PR.

@kaldoran

This comment has been minimized.

Show comment
Hide comment
@kaldoran

kaldoran Jun 28, 2016

Collaborator

Thanks, then i'll review it too :)

Collaborator

kaldoran commented Jun 28, 2016

Thanks, then i'll review it too :)

include/opendht/value.h
+ * @param id the id.
+ *
+ * @return the resulting query.
+ */

This comment has been minimized.

@kaldoran

kaldoran Jun 30, 2016

Collaborator

Wrong commentary

@kaldoran

kaldoran Jun 30, 2016

Collaborator

Wrong commentary

include/opendht/value.h
+ * @param id the id.
+ *
+ * @return the resulting query.
+ */

This comment has been minimized.

@kaldoran

kaldoran Jun 30, 2016

Collaborator

Wrong commentary too ( copy past of previous )

@kaldoran

kaldoran Jun 30, 2016

Collaborator

Wrong commentary too ( copy past of previous )

@sim590

This comment has been minimized.

Show comment
Hide comment
@sim590

sim590 Jul 10, 2016

Contributor

@kaldoran: I've updated unrelevant code commenting where needed.

@aberaud, @kaldoran: Anymore thoughts before merging this?

Contributor

sim590 commented Jul 10, 2016

@kaldoran: I've updated unrelevant code commenting where needed.

@aberaud, @kaldoran: Anymore thoughts before merging this?

@aberaud aberaud merged commit 97baacf into savoirfairelinux:master Jul 11, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@kaldoran

This comment has been minimized.

Show comment
Hide comment
@kaldoran

kaldoran Jul 12, 2016

Collaborator

Seems that your presentation at debconf help this merge 💃
Nice one anyway 👊

Collaborator

kaldoran commented Jul 12, 2016

Seems that your presentation at debconf help this merge 💃
Nice one anyway 👊

This was referenced Jul 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment