8000 fix: relieve the requirement on init file by hanxiao · Pull Request #2553 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: relieve the requirement on init file #2553

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 3, 2021
Merged

fix: relieve the requirement on init file #2553

merged 4 commits into from
Jun 3, 2021

Conversation

hanxiao
Copy link
Member
@hanxiao hanxiao commented Jun 3, 2021 •
edited
Loading

Background

prev a hub executor that introduces internal file dependencies must name its main logic file (where executor lies in) as __init__.py , more info https://github.com/jina-ai/jina-hub#-create-a-new-executor

If we don't call it as __init__.py , 1.x requires to import internal file dependencies as from jinahub.xxx, more info here https://github.com/jina-ai/jina-hub#legacy-hub-pod-structure

Both solutions in 1.x aren't perfect. __init__.py should not be required, and in general Python, it is often left empty.

This PR

This PR relieves the restriction on __init__.py and user will have the same developing & importing experience regardless if the executor is used in/out Hub.

@hanxiao hanxiao requested a review from a team as a code owner June 3, 2021 06:24
@jina-bot jina-bot added size/S 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 labels Jun 3, 2021
@codecov
Copy link
codecov bot commented Jun 3, 2021 •
edited
Loading

Codecov Report

Merging #2553 (cd8e236) into master (6ad2e00) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2553      +/-   ##
==========================================
+ Coverage   86.25%   86.29%   +0.03%     
==========================================
  Files         152      152              
  Lines        9497     9500       +3     
==========================================
+ Hits         8192     8198       +6     
+ Misses       1305     1302       -3
Flag Coverage Δ
daemon 46.64% <0.00%> (-0.02%) ⬇️
jina 86.29% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
jina/importer.py 80.69% <100.00%> (+0.29%) ⬆️
jina/clients/base.py 79.20% <0.00%> (-2.98%) ⬇️
jina/peapods/zmq/__init__.py 83.13% <0.00%> (-2.08%) ⬇️
jina/docker/hubio.py 83.49% <0.00%> (-0.48%) ⬇️
jina/peapods/pods/__init__.py 90.03% <0.00%> (+4.53%) ⬆️

Continue to review full report at Codecov.

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

@hanxiao hanxiao merged commit 33f84ab into master Jun 3, 2021
@hanxiao hanxiao deleted the fix-import-init branch June 3, 2021 06:51
JoanFM pushed a commit that referenced this pull request Jun 3, 2021
@hanxiao
fix: relieve the requirement on init file (#2553)
3ce5e95 
* fix: relieve the requirement on init file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@florian-hoenicke florian-hoenicke Awaiting requested review from florian-hoenicke

@slettner slettner Awaiting requested review from slettner

Assignees
No one assigned
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 size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hanxiao @jina-bot
0