8000 refactor: remove module from typename by jakobkruse1 · Pull Request #2486 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: remove module from typename #2486

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

Merged
merged 8 commits into from
Jun 9, 2021

Conversation

jakobkruse1
Copy link
Contributor

The following problems occurred when running examples in the 2.0 API:
When indexing or searching by yml file Flow.load_config(...), the index files are stored in

workspace/jinahub.DocVectorIndexer/0/file.json

When I do the same using the Python syntax Flow().add(...), then the index files are stored in

workspace/executors.DocVectorIndexer/0/file.json

This causes problems when you want to index via yml file, but search using the Python only interface.
This PR adresses this problem by removing the module name from the folder name.

@jakobkruse1 jakobkruse1 requested review from JoanFM and FionnD May 26, 2021 12:45
@jakobkruse1 jakobkruse1 self-assigned this May 26, 2021
@jakobkruse1 jakobkruse1 requested a review from a team as a code owner May 26, 2021 12:45
@jina-bot jina-bot added size/XS area/core This issue/PR affects the core codebase area/helper This issue/PR affects the helper functionality labels May 26, 2021
@codecov
Copy link
codecov bot commented May 26, 2021

Codecov Report

Merging #2486 (a03d5ba) into master (71a10fc) will increase coverage by 2.79%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2486      +/-   ##
==========================================
+ Coverage   84.28%   87.07%   +2.79%     
==========================================
  Files         153      153              
  Lines        9449     9449              
==========================================
+ Hits         7964     8228     +264     
+ Misses       1485     1221     -264     
Flag Coverage Δ
daemon 47.21% <100.00%> (ø)
jina 87.13% <100.00%> (+4.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/executors/__init__.py 82.65% <100.00%> (ø)
jina/peapods/pods/compound.py 89.47% <0.00%> (-1.51%) ⬇️
jina/peapods/zmq/__init__.py 83.13% <0.00%> (+0.29%) ⬆️
jina/types/message/__init__.py 87.75% <0.00%> (+1.53%) ⬆️
jina/helper.py 78.52% <0.00%> (+1.74%) ⬆️
jina/peapods/runtimes/zmq/zed.py 93.80% <0.00%> (+1.90%) ⬆️
jina/peapods/peas/helper.py 97.87% <0.00%> (+2.12%) ⬆️
jina/clients/helper.py 100.00% <0.00%> (+2.38%) ⬆️
jina/jaml/helper.py 85.12% <0.00%> (+2.47%) ⬆️
jina/clients/base.py 79.20% <0.00%> (+2.97%) ⬆️
... and 16 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 4506edd...a03d5ba. Read the comment docs.

Copy link
Contributor
@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may u add a test for this?

@jakobkruse1 jakobkruse1 requested a review from hanxiao as a code owner June 7, 2021 11:17
@jina-bot jina-bot added size/XL area/cicd This issue/PR affects the cicd pipeline area/cli This issue/PR affects the command line interface area/daemon area/docker This issue/PR affects the docker functionality area/docs This issue/PR affects the docs area/entrypoint This issue/PR affects the entrypoint codebase area/helloworld This issue/PR affects the helloworld area/housekeeping This issue/PR is housekeeping area/network This issue/PR affects network functionality area/setup This issue/PR affects setting up Jina area/testing This issue/PR affects testing component/client component/docker component/flow component/jaml and removed size/XS labels Jun 7, 2021
py_metas_name = py_executor.metas.name
py_typename = typename(py_executor)

assert jaml_metas_name == py_metas_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and what is the expected value for each of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakobkruse1 jakobkruse1 requested a review from JoanFM June 7, 2021 14:48
JoanFM
JoanFM previously approved these changes Jun 7, 2021
@JoanFM
Copy link
Contributor
JoanFM commented Jun 7, 2021

make sure u rebase with master to pass checks

@jakobkruse1 jakobkruse1 force-pushed the refactor-workspace-index-folder-names branch from 649e059 to d72f2d9 Compare June 7, 2021 16:00
@jakobkruse1
Copy link
Contributor Author

make sure u rebase with master to pass checks

I don't think the failing checks are caused by the rebase. I just rebased and the problem is still there.
Could you check the error logs, seems like there is some weird error? @JoanFM

@JoanFM JoanFM merged commit 5c29e3b into master Jun 9, 2021
@JoanFM JoanFM deleted the refactor-workspace-index-folder-names branch June 9, 2021 06:12
7BCC
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/helper This issue/PR affects the helper functionality area/testing This issue/PR affects testing component/executor executor/meta size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0