Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(executor): add use_default and with to requests field #1959

Merged
merged 3 commits into from Feb 17, 2021

Conversation

hanxiao
Copy link
Member

@hanxiao hanxiao commented Feb 17, 2021

New features for executor-level YAML

  • add use_default keyword under the requests field. If set, then missing request types and its drivers are auto-filled according to MRO.
  • add with keyword under each request type. If set, then all drivers under that request type will have the share that `kwargs.
  • when with is set under request type, then drivers keyword must be used to contain the list of drivers.

For example,

Before After
!ImageNormalizer
requests:
  on:
    ControlRequest:
      - !ControlReqDriver {}
    [IndexRequest, SearchRequest]:
      - !FilterQL
        with:
          lookups:
            mime_type: image/jpeg
          traversal_paths: ['c']
      - !URI2Blob
        with:
          traversal_paths: ['c']
      - !CraftDriver
        with:
          traversal_paths: ['c']
!ImageNormalizer
requests:
  use_default: true  # for the request_type not mentioned in `on`, fill in default requests by following MRO tree
  on:
    [IndexRequest, SearchRequest]:
      with:  # common kwargs that will be shared by all drivers
        traversal_paths: ['c']
      drivers:
        - !FilterQL {}
        - !URI2Blob {}
        - !CraftDriver {}

@hanxiao hanxiao requested a review from a team as a code owner February 17, 2021 03:47
@jina-bot jina-bot added size/M area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing component/executor component/resource executor/meta labels Feb 17, 2021
@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #1959 (4dafc09) into master (d153381) will decrease coverage by 27.95%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1959       +/-   ##
===========================================
- Coverage   86.52%   58.57%   -27.96%     
===========================================
  Files         148      148               
  Lines        7093     7094        +1     
===========================================
- Hits         6137     4155     -1982     
- Misses        956     2939     +1983     
Impacted Files Coverage Δ
jina/drivers/convertdriver.py 72.72% <ø> (-21.22%) ⬇️
jina/drivers/encode.py 33.33% <ø> (-60.87%) ⬇️
jina/drivers/evaluate.py 56.89% <ø> (-41.38%) ⬇️
jina/drivers/querylang/select.py 72.41% <ø> (-17.25%) ⬇️
jina/executors/rankers/__init__.py 58.97% <ø> (-33.34%) ⬇️
jina/parsers/check.py 85.71% <ø> (ø)
jina/parsers/client.py 100.00% <ø> (ø)
jina/parsers/export_api.py 85.71% <ø> (ø)
jina/parsers/flow.py 100.00% <ø> (ø)
jina/parsers/helloworld.py 95.00% <ø> (-5.00%) ⬇️
... and 112 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d153381...2896d75. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Feb 17, 2021

Latency summary

Current PR yields:

  • 😶 index QPS at 1197, delta to last 3 avg.: +1%
  • 🐢🐢 query QPS at 17, delta to last 3 avg.: -12%

Breakdown

Version Index QPS Query QPS
current 1197 17
1.0.0 1178 19

Backed by latency-tracking. Further commits will update this comment.

@hanxiao hanxiao merged commit 60b6dae into master Feb 17, 2021
@hanxiao hanxiao deleted the feat-improve-requests-on branch February 17, 2021 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing component/executor component/resource executor/meta size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants