Repository: jina-ai/serve
Pull request: refactor: avoid import drivers from types

Problem statement:
QueryLang instantiation with Driver instance
**Describe the bug**
When instantiating a QueryLang primitive type from a `BaseDriver` instance, we take all the parameters of that instance, and before moving to primitive types, `QueryLang` intention was to override just a single parameter.

I think this is confusing and can lead to unexpected behavior. The problem will arise when there is a driver with more than 2 overridable parameters.

Another problem is that it takes the same exact priority as the driver

Hints from issue discussion:
This issue is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 4 days
This issue is stale because it has been open 20 days with no activity. Remove stale label or comment or this will be closed in 4 days

Commit message: Merge branch 'master' of github.com:jina-ai/jina into refactor-decouple-types-drivers

Patch to review:
diff --git a/jina/clients/request.py b/jina/clients/request.py
index 48819e5995d9e..2709225aad2f3 100644
--- a/jina/clients/request.py
+++ b/jina/clients/request.py
@@ -112,11 +112,8 @@ def search(*args, **kwargs):
     """Generate a searching request """
     if ('top_k' in kwargs) and (kwargs['top_k'] is not None):
         # associate all VectorSearchDriver and SliceQL driver to use top_k
-        # TODO: not really elegant, to be refactored (Han)
-        from ..drivers.querylang.slice import SliceQL
-        from ..drivers.search import VectorSearchDriver
-        topk_ql = [QueryLang(SliceQL(start=0, end=kwargs['top_k'], priority=1)),
-                   QueryLang(VectorSearchDriver(top_k=kwargs['top_k'], priority=1))]
+        topk_ql = [QueryLang(('SliceQL', {'end': kwargs['top_k'], 'priority': 1})),
+                   QueryLang(('VectorSearchDriver', {'top_k': kwargs['top_k'], 'priority': 1}))]
         if 'queryset' not in kwargs:
             kwargs['queryset'] = topk_ql
         else:
diff --git a/jina/types/querylang/__init__.py b/jina/types/querylang/__init__.py
index 9e4f6b4529510..954d6017743f8 100644
--- a/jina/types/querylang/__init__.py
+++ b/jina/types/querylang/__init__.py
@@ -1,17 +1,15 @@
 import warnings
-from typing import TypeVar, Dict, Optional, Type
+from typing import TypeVar, Dict, Optional, Tuple
 
 from google.protobuf import json_format
 
-from ...drivers import BaseDriver
 from ...excepts import BadQueryLangType
 from ...helper import typename
-from ...importer import import_classes
 from ...proto import jina_pb2
 
 
 QueryLangSourceType = TypeVar('QueryLangSourceType',
-                              jina_pb2.QueryLangProto, bytes, str, Dict, BaseDriver)
+                              jina_pb2.QueryLangProto, bytes, str, Dict, Tuple[str, int, dict])
 
 __all__ = ['QueryLang']
 
@@ -23,7 +21,8 @@ class QueryLang:
     It offers a Pythonic interface to allow users access and manipulate
     :class:`jina.jina_pb2.QueryLangProto` object without working with Protobuf itself.
 
-    To create a :class:`QueryLang` object from a :class:`BaseDriver` object, simply:
+    To create a :class:`QueryLang` object from a Tuple containing the name of a :class:`BaseDriver`,
+     and the parameters to override, simply:
 
         .. highlight:: python
         .. code-block:: python
@@ -32,12 +31,15 @@ class QueryLang:
             from jina.drivers.querylang.slice import SliceQL
 
             s = SliceQL(start=3, end=4)
-            ql = QueryLang(s)
+            ql = QueryLang('SliceQL', 1, {'start': 3, 'end': 1, 'priority': 1})
+
+    .. warning::
+        The `BaseDriver` needs to be a `QuerySetReader` to be able to read the `QueryLang`
 
     One can also build a :class`QueryLang` from JSON string, bytes, dict or directly from a protobuf object.
 
     A :class:`QueryLang` object (no matter how it is constructed) can be converted to
-    protobuf object or back to driver object by using:
+    protobuf object by using:
 
         .. highlight:: python
         .. code-block:: python
@@ -45,18 +47,13 @@ class QueryLang:
             # to protobuf object
             s.as_pb_object
 
-            # to driver object
-            s.as_driver_object
-
-    To get the class name of the associated driver, one can use :attr:`driver`.
-
     """
 
     def __init__(self, querylang: Optional[QueryLangSourceType] = None, copy: bool = False):
         """
 
         :param querylang: the query language source to construct from, acceptable types include:
-            :class:`jina_pb2.QueryLangProto`, :class:`bytes`, :class:`str`, :class:`Dict`, :class:`BaseDriver`.
+            :class:`jina_pb2.QueryLangProto`, :class:`bytes`, :class:`str`, :class:`Dict`, Tuple.
         :param copy: when ``querylang`` is given as a :class:`QueryLangProto` object, build a
                 view (i.e. weak reference) from it or a deep copy from it.
         """
@@ -84,10 +81,10 @@ def __init__(self, querylang: Optional[QueryLangSourceType] = None, copy: bool =
                         self._querylang.ParseFromString(querylang)
                     except RuntimeWarning as ex:
                         raise BadQueryLangType('fail to construct a query language') from ex
-            elif isinstance(querylang, BaseDriver):
-                self.driver = querylang
-                self.priority = querylang._priority
-                self._querylang.parameters.update(querylang._init_kwargs_dict)
+            elif isinstance(querylang, Tuple):
+                self.name = querylang[0]
+                self._querylang.priority = querylang[1]['priority'] if 'priority' in querylang[1] else 0
+                self._querylang.parameters.update(querylang[1])
             elif querylang is not None:
                 # note ``None`` is not considered as a bad type
                 raise ValueError(f'{typename(querylang)} is not recognizable')
@@ -114,21 +111,6 @@ def name(self, value: str):
         """Set the name of the driver that the query language attached to """
         self._querylang.name = value
 
-    @property
-    def driver(self) -> Type['BaseDriver']:
-        """Get the driver class that the query language attached to
-
-        ..warning::
-            This browses all module trees and can be costly,
-            do not frequently call it.
-        """
-        return import_classes('jina.drivers', targets=[self.name])
-
-    @driver.setter
-    def driver(self, value: 'BaseDriver'):
-        """Set the driver class that the query language attached to """
-        self._querylang.name = value.__class__.__name__
-
     def __getattr__(self, name: str):
         return getattr(self._querylang, name)
 
@@ -136,8 +118,3 @@ def __getattr__(self, name: str):
     def as_pb_object(self) -> 'jina_pb2.QueryLangProto':
         """Return a protobuf :class:`jina_pb2.QueryLangProto` object """
         return self._querylang
-
-    @property
-    def as_driver_object(self) -> 'BaseDriver':
-        """Return a :class:`BaseDriver` object """
-        return self.driver(**self._querylang.parameters)
diff --git a/jina/types/sets/querylang.py b/jina/types/sets/querylang.py
index 0ad7bcb1e0706..22b2776122a71 100644
--- a/jina/types/sets/querylang.py
+++ b/jina/types/sets/querylang.py
@@ -1,14 +1,13 @@
 from collections.abc import MutableSequence
-from typing import Iterable, Union
+from typing import Iterable, Union, Tuple
 
 from google.protobuf.pyext._message import RepeatedCompositeContainer
 
 from ..querylang import QueryLang
-from ...drivers import BaseDriver
 from ...helper import typename
 from ...proto.jina_pb2 import QueryLangProto
 
-AcceptQueryLangType = Union[QueryLang, BaseDriver, QueryLangProto]
+AcceptQueryLangType = Union[QueryLang, Tuple, QueryLangProto]
 
 __all__ = ['QueryLangSet', 'AcceptQueryLangType']
 
@@ -55,7 +54,7 @@ def __getitem__(self, item):
 
     def append(self, value: 'AcceptQueryLangType'):
         q_pb = self._querylangs_proto.add()
-        if isinstance(value, BaseDriver):
+        if isinstance(value, Tuple):
             q_pb.CopyFrom(QueryLang(value).as_pb_object)
         elif isinstance(value, QueryLangProto):
             q_pb.CopyFrom(value)
diff --git a/tests/integration/issues/github_1072/test_queryset.py b/tests/integration/issues/github_1072/test_queryset.py
index 585f51aa16926..66a4c0c8392ed 100644
--- a/tests/integration/issues/github_1072/test_queryset.py
+++ b/tests/integration/issues/github_1072/test_queryset.py
@@ -4,7 +4,6 @@
 import pytest
 
 from jina import QueryLang
-from jina.drivers.querylang.filter import FilterQL
 from jina.flow import Flow
 from jina.proto import jina_pb2
 from jina.types.ndarray.generic import NdArray
@@ -46,7 +45,7 @@ def validate_label2_docs(resp):
         # keep all the docs
         f.index(docs, on_done=validate_all_docs, callback_on='body')
         # keep only the docs with label2
-        qs = QueryLang(FilterQL(priority=1, lookups={'tags__label': 'label2'}, traversal_paths=['r']))
+        qs = QueryLang(('FilterQL', {'priority': 1, 'lookups': {'tags__label': 'label2'}, 'traversal_paths': ['r']}))
         f.index(docs, queryset=qs, on_done=validate_label2_docs, callback_on='body')
 
     mock1.assert_called_once()
diff --git a/tests/integration/issues/github_976/test_topk.py b/tests/integration/issues/github_976/test_topk.py
index 7d97b74d07991..52b871d5cd32f 100644
--- a/tests/integration/issues/github_976/test_topk.py
+++ b/tests/integration/issues/github_976/test_topk.py
@@ -4,7 +4,6 @@
 import pytest
 
 from jina import QueryLang
-from jina.drivers.search import VectorSearchDriver
 from jina.flow import Flow
 from jina.proto import jina_pb2
 from jina.types.ndarray.generic import NdArray
@@ -59,10 +58,8 @@ def validate(resp):
         for doc in resp.search.docs:
             assert len(doc.matches) == TOPK_OVERRIDE
 
-
-
     # Making queryset
-    top_k_queryset = QueryLang(VectorSearchDriver(top_k=TOPK_OVERRIDE, priority=1))
+    top_k_queryset = QueryLang(('VectorSearchDriver', {'top_k': TOPK_OVERRIDE, 'priority': 1}))
 
     with Flow.load_config('flow.yml') as index_flow:
         index_flow.index(input_fn=random_docs(100))
diff --git a/tests/unit/drivers/querylang/test_querylang_reader.py b/tests/unit/drivers/querylang/test_querylang_reader.py
index 5fc3ae0c3d213..7b6bc9232740b 100644
--- a/tests/unit/drivers/querylang/test_querylang_reader.py
+++ b/tests/unit/drivers/querylang/test_querylang_reader.py
@@ -1,6 +1,5 @@
 from jina.clients import Client
 from jina.drivers import QuerySetReader, BaseDriver
-from jina.drivers.querylang.slice import SliceQL
 from jina.flow import Flow
 from jina.proto import jina_pb2
 from jina.types.querylang import QueryLang
@@ -36,7 +35,7 @@ def __init__(self, arg1='hello', arg2=456, *args, **kwargs):
 
 
 def test_querylang_request():
-    qs = QueryLang(SliceQL(start=1, end=4, priority=1))
+    qs = QueryLang(('SliceQL', {'start': 1, 'end': 4, 'priority': 1}))
     Client.check_input(random_docs(10), queryset=qs)
 
 
@@ -51,7 +50,7 @@ def validate2(req):
     response_mock_2 = mocker.Mock(wrap=validate2)
     response_mock_3 = mocker.Mock(wrap=validate1)
 
-    qs = QueryLang(SliceQL(start=1, end=4, priority=1))
+    qs = QueryLang(('SliceQL', {'start': 1, 'end': 4, 'priority': 1}))
 
     f = Flow(callback_on='body').add(uses='- !SliceQL | {start: 0, end: 5}')
 
diff --git a/tests/unit/types/message/test_message.py b/tests/unit/types/message/test_message.py
index bc7548e03e8fb..edd6ee3c40191 100644
--- a/tests/unit/types/message/test_message.py
+++ b/tests/unit/types/message/test_message.py
@@ -5,7 +5,6 @@
 
 from jina import Request, QueryLang, Document
 from jina.clients.request import _generate
-from jina.drivers.querylang.slice import SliceQL
 from jina.proto import jina_pb2
 from jina.proto.jina_pb2 import EnvelopeProto
 from jina.types.message import Message
@@ -131,8 +130,8 @@ def test_lazy_request_fields():
 
 
 def test_request_extend_queryset():
-    q1 = SliceQL(start=3, end=4)
-    q2 = QueryLang(SliceQL(start=3, end=4, priority=1))
+    q1 = ('SliceQ', {'start': 3, 'end': 4})
+    q2 = QueryLang(('SliceQL', {'start': 3, 'end': 4, 'priority': 1}))
     q3 = jina_pb2.QueryLangProto()
     q3.name = 'SliceQL'
     q3.parameters['start'] = 3
@@ -141,6 +140,7 @@ def test_request_extend_queryset():
     r = Request()
     r.queryset.extend([q1, q2, q3])
     assert isinstance(r.queryset, Sequence)
+    assert len(r.queryset) == 3
     for idx, q in enumerate(r.queryset):
         assert q.priority == idx
         assert q.parameters['start'] == 3
@@ -180,6 +180,7 @@ def test_empty_request_type(typ, pb_typ):
     assert r._request_type == typ
     assert isinstance(r.body, pb_typ)
 
+
 @pytest.mark.parametrize('typ,pb_typ', [('index', jina_pb2.RequestProto.IndexRequestProto),
                                         ('search', jina_pb2.RequestProto.SearchRequestProto)])
 def test_add_doc_to_type(typ, pb_typ):
diff --git a/tests/unit/types/querylang/test_querylang.py b/tests/unit/types/querylang/test_querylang.py
index c820b6ee398e8..8aa3d18c18067 100644
--- a/tests/unit/types/querylang/test_querylang.py
+++ b/tests/unit/types/querylang/test_querylang.py
@@ -1,13 +1,11 @@
 import pytest
 from google.protobuf.json_format import MessageToDict, MessageToJson
 
-from jina.drivers.querylang.slice import SliceQL
 from jina.types.querylang import QueryLang
 
 
-def test_ql_constructors_from_driver():
-    ql = SliceQL(start=3, end=5, priority=999)
-    q = QueryLang(ql)
+def test_ql_constructors_from_driver_info():
+    q = QueryLang(('SliceQL', {'start': 3, 'end': 5, 'priority': 999}))
     qb = q.as_pb_object
     assert q.name == 'SliceQL'
     assert q.parameters['start'] == 3
@@ -19,19 +17,13 @@ def test_ql_constructors_from_driver():
     assert qb.parameters['end'] == 5
     assert qb.priority == 999
 
-    assert isinstance(q.as_driver_object, SliceQL)
-    assert ql.start == q.as_driver_object.start
-    assert ql.end == q.as_driver_object.end
-    assert ql._priority == q.as_driver_object._priority
-
 
 @pytest.mark.parametrize('source', [lambda x: x.SerializeToString(),
                                     lambda x: MessageToDict(x),
                                     lambda x: MessageToJson(x),
                                     lambda x: x])
 def test_ql_constructors_from_proto(source):
-    ql = SliceQL(start=3, end=5, priority=999)
-    q = QueryLang(ql).as_pb_object
+    q = QueryLang(('SliceQL', {'start': 3, 'end': 5, 'priority': 999})).as_pb_object
 
     qlr = QueryLang(source(q))
     assert qlr.name == 'SliceQL'
@@ -41,7 +33,7 @@ def test_ql_constructors_from_proto(source):
 
 
 def test_ql_priority():
-    qs = QueryLang(SliceQL(start=1, end=4, priority=1))
+    qs = QueryLang(('SliceQL', {'start': 1, 'end': 4, 'priority': 1}))
     assert qs.priority == 1
     qs._querylang.priority = -1
     assert qs._querylang.priority == -1
@@ -50,3 +42,7 @@ def test_ql_priority():
     qs.priority = -2
     assert qs._querylang.priority == -2
     assert qs.priority == -2
+
+    qs2 = QueryLang(('SliceQL', {'start': 1, 'end': 4}))
+    assert qs2.priority == 0
+
diff --git a/tests/unit/types/sets/test_querylangset.py b/tests/unit/types/sets/test_querylangset.py
index 5035c5d935f56..fe6f0ae1b9c39 100644
--- a/tests/unit/types/sets/test_querylangset.py
+++ b/tests/unit/types/sets/test_querylangset.py
@@ -4,6 +4,7 @@
 from jina.types.querylang import QueryLang
 from jina.types.sets.querylang import QueryLangSet
 
+
 @pytest.fixture(scope='function')
 def querylang_protos():
     """:returns: A :class:`RepeatedCompositeContainer` consist list of :class:`QueryLangProto`."""
@@ -83,8 +84,8 @@ def test_iter(querylang_set):
         assert isinstance(querylang, QueryLang)
 
 
-@pytest.mark.parametrize('querylang_item', [QueryLangProto(), QueryLang()])
-def test_append_success(querylang_set, querylang_item):
+@pytest.mark.parametrize('querylang_item', [QueryLangProto(), QueryLang(), ('Driver', {'key': 'value'})])
+def test_append_success_proto(querylang_set, querylang_item):
     """Test :meth:`append`. Expect test three cases depends on the type of :attr:`value`.
     Such as :class:`BaseDriver`, :class:`QueryLangProto` and :class:`QueryLang`.
 


Code review comments:
diff_hunk: @@ -32,31 +31,29 @@ class QueryLang:
             from jina.drivers.querylang.slice import SliceQL
 
             s = SliceQL(start=3, end=4)
-            ql = QueryLang(s)
+            ql = QueryLang('SliceQL', 1, {'start': 3, 'end': 1, 'priority': 1})
+
+    .. warning::
+        The `BaseDriver` needs to be a `QuerySetReader` to be able to read the `QueryLang`
 
     One can also build a :class`QueryLang` from JSON string, bytes, dict or directly from a protobuf object.
 
     A :class:`QueryLang` object (no matter how it is constructed) can be converted to
-    protobuf object or back to driver object by using:
+    protobuf object by using:
 
         .. highlight:: python
         .. code-block:: python
 
             # to protobuf object
             s.as_pb_object
path: jina/types/querylang/__init__.py
text: @user1:
```suggestion
            ql.as_pb_object
```

diff_hunk: @@ -32,31 +31,29 @@ class QueryLang:
             from jina.drivers.querylang.slice import SliceQL
 
             s = SliceQL(start=3, end=4)
path: jina/types/querylang/__init__.py
text: @user1:
```suggestion
```