-
Notifications
You must be signed in to change notification settings - Fork 155
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
DataError on to_alias field name (254 > 40) #7
Comments
Yeah, field should be expanded to 254 (which is the RFC standard). Feel free to submit a PR with migrations, or I'll get to it when I can. |
Steps to recreate issue? |
sorry, absolutely forgot about the issue :) created PR with test, which should be failed with current model, but it isn't. i also checked that resulting |
hm, interesting. i don't have much time to dig into this right now but i'll check in on it on a weekend when I have time if somebody else doesn't find out why that test is passing |
I'd like to take a crack at it. Could I use your branch? |
sure
ср, 17 апр. 2019 г., 8:19 Mohammed Ali Zubair <notifications@github.com>:
… sorry, absolutely forgot about the issue :) created PR with test, which
should be failed with current model, but it isn't. i also checked that
resulting to_alias value will be equal to email, which is longer than
field defined in model self.assertEqual(callback_token.to_alias, email)
https://travis-ci.org/aaronn/django-rest-framework-passwordless/jobs/520698631
@aaronn <https://github.com/aaronn> @Alig1493
<https://github.com/Alig1493> maybe somebody is able to help with?
I'd like to take a crack at it. Could I use your branch?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AMtpTf7MFNkZRPYCy48APVYvnBS8RVoVks5vhq7FgaJpZM4R3TCe>
.
|
@roman-karpovich I forked and created a branch for myself hope that's okay. You can take a look I referenced this issue. I made a small change in the model save because django doesn't invoke model level validation on either save or create. As a result I had to call full_clean overriding the save method. |
thanks! interesting, i thought that at least integrity error should be
raised by database.
ср, 17 апр. 2019 г., 8:27 Mohammed Ali Zubair <notifications@github.com>:
… @roman-karpovich <https://github.com/roman-karpovich> I forked and
created a branch for myself hope that's okay. You can take a look I
referenced this issue. I made a small change in the model save because
django doesn't invoke model level validation on either save or create. As a
result I had to call full_clean overriding the save method.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AMtpTZut19xK9yV_fVba2njl4BHpDCFMks5vhrDWgaJpZM4R3TCe>
.
|
ok. anyway, models should be changed. does easy way exists to create alter field migration without manage.py script? |
I think we need to migrate for sure– why is that test you folks added passing? Very weird. |
Release 1.4+ fixes this by adding max_length=254. |
drfpasswordless can't be used with contrib.auth.User model, because
AbstractBaseCallbackToken.to_alias
field, which in theory should contain user email, is just 40 symbols length, while default value for user email max_length = 254.i guess, field should be expanded, or at least cropped before saving (if custom user model will be used with even longer values).
The text was updated successfully, but these errors were encountered: