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

Using list with host parameter in blueprint.route() does not work. #1772

Closed
damianj opened this issue Jan 26, 2020 · 4 comments
Closed

Using list with host parameter in blueprint.route() does not work. #1772

damianj opened this issue Jan 26, 2020 · 4 comments

Comments

@damianj
Copy link
Contributor

damianj commented Jan 26, 2020

Describe the bug
As the title implies, when I try to set the host parameter to a list on a blueprint route, the app does not start and instead gives an error like:

Traceback (most recent call last):
  File "test.py", line 16, in <module>
    parent_app.blueprint(child_bp)
  File "/usr/lib/python3.8/site-packages/sanic/app.py", line 757, in blueprint
    blueprint.register(self, options)
  File "/usr/lib/python3.8/site-packages/sanic/blueprints.py", line 146, in register
    route_names = [route.name for route in routes]
  File "/usr/lib/python3.8/site-packages/sanic/blueprints.py", line 146, in <listcomp>
    route_names = [route.name for route in routes]
AttributeError: 'NoneType' object has no attribute 'name'

Code snippet
To reproduce that error:

from sanic import Sanic, Blueprint
from sanic.response import text

child_bp = Blueprint(name='child_blueprint', url_prefix='/')
parent_app = Sanic(__name__)


@child_bp.route('/', host=["localhost:8000", "127.0.0.1:8000"])
async def hello_world(request):
    return text("Hello world!")

parent_app.blueprint(child_bp)
parent_app.run()

Note that if host is set to a string (i.e., @child_bp.route('/', host="127.0.0.1:8000")) or it's not even specified in the route (i.e., @child_bp.route('/')) then the app runs just fine.

Expected behavior
I would expect that the host parameter would function similar to that described in https://sanic.readthedocs.io/en/latest/sanic/routing.html#http-request-types

There is also an optional host argument (which can be a list or a string). This restricts a route to the host or hosts provided. If there is a also a route with no host, it will be the default.

Environment (please complete the following information):

  • OS: Arch Linux (Kernel 5.4.13)
  • Version: 19.12.2

Additional context
This was working previously so I don't know if it's a bug or some intentional change in functionality.

@damianj damianj changed the title Using list with host argument in blueprint.route() does not work. Using list with host parameter in blueprint.route() does not work. Jan 26, 2020
damianj added a commit to damianj/sanic that referenced this issue Jan 30, 2020
As explained in sanic-org#1772 there is an issue when using a list as an argument for the host parameter in the Blueprint.route() decorator. I've traced the issue back to this line, and the if conditional should ensure that the name attribute isn't accessed when route is None.
@damianj
Copy link
Contributor Author

damianj commented Jan 30, 2020

I've opened #1776 to address this issue.

@ahopkins ahopkins reopened this Jan 30, 2020
@ahopkins
Copy link
Member

Thanks for bringing this up.

@stale
Copy link

stale bot commented Apr 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@stale stale bot added the stale label Apr 29, 2020
ahopkins added a commit that referenced this issue Jun 28, 2020
* Bug fix for host parameter issue with lists

As explained in #1772 there is an issue when using a list as an argument for the host parameter in the Blueprint.route() decorator. I've traced the issue back to this line, and the if conditional should ensure that the name attribute isn't accessed when route is None.

* Unit tests for blueprint.route host paramter set to list.

Co-authored-by: Adam Hopkins <[email protected]>
@myusko
Copy link
Contributor

myusko commented Sep 25, 2020

@ahopkins @ashleysommer probably we should close the issue since we already merged PR with a fix.

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

No branches or pull requests

3 participants