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

Add missing migration for webhooks #5078

Merged
merged 4 commits into from
Oct 12, 2022
Merged

Conversation

zhiltsov-max
Copy link
Contributor

@zhiltsov-max zhiltsov-max commented Oct 11, 2022

Motivation and context

It seems that a migration was done in #4863 , but it was not added to the repository. This PR adds the missing migration, which is generated by python manage.py makemigrations.

  • Changed the webhook model status_code field type

How has this been tested?

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@sizov-kirill
Copy link
Contributor

Added migration quite similar with this migration.

And the reason why the lists of available values differ is because of the version of Python being used. Because for different python version http.HTTPStatus can differ.

So I think we should make WebhookDelivery.status_code independent from Python version.

And for now I see only one way to do it: hardcode all possible status codes. @zhiltsov-max what do you think about it?

@zhiltsov-max
Copy link
Contributor Author

I think we can just store an integer without extra restrictions.

@zhiltsov-max zhiltsov-max changed the title Add missing migration for webhooks [WIP] Add missing migration for webhooks Oct 11, 2022
@nmanovic
Copy link
Contributor

/check

@github-actions
Copy link
Contributor

github-actions bot commented Oct 11, 2022

❌ Some checks failed
📄 See logs here

@zhiltsov-max zhiltsov-max changed the title [WIP] Add missing migration for webhooks Add missing migration for webhooks Oct 11, 2022
@zhiltsov-max
Copy link
Contributor Author

/check

@github-actions
Copy link
Contributor

github-actions bot commented Oct 12, 2022

❌ Some checks failed
📄 See logs here

@zhiltsov-max zhiltsov-max merged commit 1b807fb into develop Oct 12, 2022
@zhiltsov-max zhiltsov-max deleted the zm/add-webhook-migration branch October 12, 2022 08:31
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