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

Update __init__.py #654

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update __init__.py #654

wants to merge 2 commits into from

Conversation

kanhebei
Copy link

@kanhebei kanhebei commented Jun 1, 2024

The original

   _The first method:_
    from blueprint import bp
    api = Api(app)
    api.register_bluepirnt(bp)
_The second method_
form blueprint import bp
api = Api()
# # Must call init_app before registering the API blueprint
api.init_app(app)

api.register_blueprint(bp)    

After the update, there are no issues with the above two writing methods, and the following new usage methods will continue to be added

api = Api()

from blueprint import bp
api.register_blueprint(bp)
# #Or register as a string
api.register_blueprint('blueprint.bp')

# # last
api.init_app(app)

Copy link

codecov bot commented Jun 1, 2024

Codecov Report

Attention: Patch coverage is 64.70588% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 99.21%. Comparing base (fa5bb5d) to head (da8b518).
Report is 15 commits behind head on main.

Files Patch % Lines
src/flask_smorest/__init__.py 64.70% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #654      +/-   ##
==========================================
- Coverage   99.88%   99.21%   -0.68%     
==========================================
  Files          14       14              
  Lines         881      890       +9     
  Branches      192      195       +3     
==========================================
+ Hits          880      883       +3     
- Misses          0        3       +3     
- Partials        1        4       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lafrech
Copy link
Member

lafrech commented Jun 2, 2024

I get your point.

This is not strictly needed. See for instance in this app how we register blueprints after initializing api: https://github.com/BEMServer/bemserver-api/blob/main/src/bemserver_api/__init__.py. But I don't mind adding this.

Regarding importing as string, I don't mind either. Since the work is done in werkzeug, it is not too much API surface added.

Don't bother adding type hints. There is no CI check for them yet. However, anyone is welcome to add type hints to the code base in another PR.

I'd move the part that does the registration to a dedicated _register_blueprint method that would be called at __init__ (no need to call import_string and check self._app at this stage). Not a performance issue, but to make the intent clear.

@pat-jpnk
Copy link

Don't bother adding type hints. There is no CI check for them yet. However, anyone is welcome to add type hints to the code base in another PR.

You mean it would make sense to add it once the whole code base is covered?

@lafrech
Copy link
Member

lafrech commented Jul 16, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants