Aiohttp: url_for should cope with non str values

[samuelcolvin]

Long story short

I'm migrating an app from aiohttp 2.x to 3.0.5. The only confusing problem I've found is that url_for now throws an error if an int (and presumably any other non str value) is passed

Expected behaviour

.url_for(id=123) should return the appropriate url without an error.

Actual behaviour

Error raised

>       url = cli.server.app.router['contractor-get'].url_for(company='thepublickey', id=1, slug='x')

tests/test_public_views.py:221: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
env/lib/python3.6/site-packages/aiohttp/web_urldispatcher.py:422: in url_for
    for k, v in parts.items()})
env/lib/python3.6/site-packages/aiohttp/web_urldispatcher.py:422: in <dictcomp>
    for k, v in parts.items()})
env/lib/python3.6/site-packages/yarl/__init__.py:205: in build
    path = cls._PATH_QUOTER(path)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   ???
E   TypeError: Argument should be str

yarl/_quoting.pyx:226: TypeError

That's fixed if I use id='1'

Perhaps this should go on yarl but it appears to be related to the aioohttp upgrade, not the yarl upgrade.

Can give more details if required.

If it helps the actual pull request fixing this is open source: tutorcruncher/socket-server#161

[asvetlov]

url_for() is a reverse for match_info. aiohttp doesn't support type conversion for mapped variables, why should we support it for URL building?
The feature was never documented. Moreover implementation was a little buggy (requote did not apply)

[samuelcolvin]

I understand that in theory this could lead odd behaviour, eg. how many decimal places to use when casting a float to str.

However practically I think ints are very common arguments in urls, floats, bytes, bools etc are not.

How about _PATH_QUOTER supports int and str but nothing else? int is both very common and generally unambiguous as to how it would be converted.

[asvetlov]

URL path is string, path segments are strings also.
URL(a) / 1234 looks odd for me.

[samuelcolvin]

I agree URL(a) / 1234 looks odd, but to me .url_for(id=str(user_id)) looks odd too.

Could we allow ints in url_for only?

[harobed]

I don't want to put |string everywhere 😥

For instance:

    <li><a href="{{ url('poll', question_id=question.id|string) }}">{{ question.question_text }}</a></li>

[asvetlov]

aiohttp_jinja2 is totally different story, even casting everything to string make sense maybe. We can discuss it in separate issue for the project.
I have a feeling that supporting int by url_for() makes a mess but could live with it if the method will generate a warning for int arguments.

[harobed]

We can discuss it in separate issue for the project.

👍 where?

[samuelcolvin]

the warning way I think is ugly, either it should work or not.

I can live with requiring str, I'll just always use a function like this.

+1 where?

on https://github.com/aio-libs/aiohttp-jinja2

[asvetlov]

Let's close the issue than

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].