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 support for Cornice 2, dropping support for Cornice 1. #1958

Merged
merged 1 commit into from Nov 8, 2017

Conversation

bowlofeggs
Copy link
Contributor

Cornice 2 requires pyramid >= 1.7 and Fedora 25 only has
pyramid-1.5, so this commit also adjusts the Vagrantfile.example
to use Fedora 26 by default instead of 25.

re #1922

Signed-off-by: Randy Barlow randy@electronsweatshop.com

@bowlofeggs bowlofeggs added API Issues related to Bodhi's REST API High priority These issues are higher priority than normal Refactor Issues that are a refactor to improve maintainability for Bodhi labels Oct 30, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Oct 30, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Oct 31, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Oct 31, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Oct 31, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Oct 31, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 3, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 3, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 3, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 3, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 3, 2017
@fedora-infra fedora-infra deleted a comment from centos-ci Nov 4, 2017
Copy link
Member

@jeremycline jeremycline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with cornice or the differences between 1 and 2, but the change here looks okay.

@@ -14,6 +14,7 @@

from datetime import datetime, timedelta

from cornice.validators import _colander
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. Private APIs :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremycline Perhaps it will assuage your fears to know that I've only used this private API temporarily until there is an upstream release that has the _generate_colander_validator() that I wrote below.

I've pitched that code upstream and I have the PR approved, but it needs some tests:

Cornices/cornice#465

So it's only a private API because I wanted to use the exact same thing that will eventually be in an upstream Cornice release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and actually, the public API that will ultimately be upstream will actually be the colander_querystring_validator() that is made by _generate_colander_validator(). So once this is upstream, I will remove the code from this file and just use it directly from cornice.

- python2-sqlalchemy_schemadisplay
- python2-waitress
- python2-yaml
- python2-zmq
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why do you need to directly depend on zmq? fedmsg should depend on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why zmq is here, but I just tested the Vagrant box without it and it worked fine, so I'll drop it.

Cornice 2 requires pyramid >= 1.7 and Fedora 25 only has
pyramid-1.5, so this commit also adjusts the Vagrantfile.example
to use Fedora 26 by default instead of 25.

re fedora-infra#1922

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
@bowlofeggs bowlofeggs merged commit 803f692 into fedora-infra:develop Nov 8, 2017
@bowlofeggs bowlofeggs deleted the 1922 branch November 8, 2017 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Issues related to Bodhi's REST API High priority These issues are higher priority than normal Refactor Issues that are a refactor to improve maintainability for Bodhi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants