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

Don't overwrite marshmallow URL #6

Closed
svenstaro opened this issue Dec 1, 2014 · 5 comments

Comments

@svenstaro
Copy link

commented Dec 1, 2014

Currently you are overwriting marshmallow's own URL field. I don't think this is a good choice generally because in my case I want to validate a URL (so I have an input schema) and obviously I don't have an endpoint to provide it with in this case. It doesn't even make sense to have URLs require endpoints for input/validation.

Perhaps you should call the URL within flask-marshmallow differently?

@sloria

This comment has been minimized.

Copy link
Member

commented Dec 4, 2014

You could can still use the vanilla marshmallow URL field:

from marshmallow.fields import URL as ValidatedURL

class InSchema(ma.Schema):
    url = ValidatedURL()

This makes me think, though: flask-marshmallow's URL field currently does not have any validation or deserialization behavior. It may make sense for it to have the same validation behavior as vanilla marshmallow's URL field.

@svenstaro

This comment has been minimized.

Copy link
Author

commented Dec 4, 2014

I think the URL field within flask-marshmallow should be called differently in any case. Currently it's the only field that overwrites a regular marshmallow field which introduces inconsistency. It should be named after its purpose which is to provide a url_for to the serializer. Also, there is probably no need to deserialize this kind of URL and validate it.

What would that even look like? Check that it is a valid flask route?

@sloria

This comment has been minimized.

Copy link
Member

commented Dec 6, 2014

OK, how about renaming to URLFor or Link?

You are correct; this type of field does not not need deserialization/validation behavior.

@svenstaro

This comment has been minimized.

Copy link
Author

commented Dec 7, 2014

I'd name it URLFor because that's essentially what it does.

@sloria

This comment has been minimized.

Copy link
Member

commented Dec 7, 2014

👍

@sloria sloria closed this in f9d5ceb Dec 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.