Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1667718 - Review Request: python2-jsonschema - JSON Schema validation implementation for Python
Summary: Review Request: python2-jsonschema - JSON Schema validation implementation fo...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Miro Hrončok
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-01-20 11:34 UTC by Fabio Valentini
Modified: 2019-01-22 15:15 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-01-22 15:15:44 UTC
Type: ---
Embargoed:
mhroncok: fedora-review+

Attachments (Terms of Use)

Description Fabio Valentini 2019-01-20 11:34:16 UTC 
Spec URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema.spec
SRPM URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema-2.6.0-7.fc29.src.rpm

Description:
jsonschema is JSON Schema validator currently based on
http://tools.ietf.org/html/draft-zyp-json-schema-03

Fedora Account System Username: decathorpe

koji scratch build for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=32141679


This package is based on the current python2 sub-package of python-jsonschema, with some updates for current Packaging Guidelines, and a patch to fix the test suite (also fixing the current F29+ FTBFS issue).


=========
IMPORTANT
=========

This package is introduced in preparation for dropping the python2 sub-package from the already existing jsonschema package in rawhide *only*, which won't be able to include a python2 sub-package with version 3.0.0, because:

The next version of jsonschema (3.0.0) will introduce dependencies for which the python2 versions have already been retired (e.g. pyrsistent). So, the plan is to remove the python2 sub-package of jsonschema alongside the update to 3.0.0, and provide version 2.6.0 of the python2 package with this package.

The python2-jsonschema package can't just be dropped yet, because there are still some dependents in rawhide, according to mhroncok (who gave me good advice on how to pull off this change correctly):

  fts-rest
  gofed
  kimchi
  pungi
  python-falcon
  python-nbformat
  python-openstacksdk
  python-proliantutils
  python-warlock
Comment 1 Miro Hrončok 2019-01-20 12:04:26 UTC 
1. Note that there is also python2-backports-functools_lru_cache which IMHO makes more sense than using python2-repoze-lru. However both packages are still needed by stuff in Fedora so switching to one or another won't unblock the remaining one, so this is not important.

2. Are the RHEL6 conditionals worth it? Will the package even build on RHEL6? Are we planning to change anything in EPEL6 about this? I hope we are not.


3. The manual "Requires: python2-repoze-lru" is not needed, the patch patches setup.py and hence the package requires python2.7dist(repoze.lru).

4. Is %{_bindir}/jsonschema-2 needed? Can we not ship it and only consider this a compat library package?

5. I suggest to use %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info/ with the leading slash so no surprises happen if it happens to be a directory. 

6. setup.py uses setuptools, BR python2-setuptools explicitly (the transitive dependency can go away in the future, see python2.spec for details).


Everything else looks good.

Note A: Depends on deprecated python2 but is a rename, so ti si allowed.

Note B: It is a rename, but no binary package rename, so no new provides/obsoletes are needed.


CHECKSUM(SHA256) this package     : 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02
CHECKSUM(SHA256) upstream package : 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02


Rpmlint
-------
Checking: python2-jsonschema-2.6.0-7.fc30.noarch.rpm
          python2-jsonschema-2.6.0-7.fc30.src.rpm
python2-jsonschema.noarch: W: spelling-error %description -l en_US validator -> lavatorial
python2-jsonschema.noarch: W: no-manual-page-for-binary jsonschema-2
python2-jsonschema.src: W: spelling-error %description -l en_US validator -> lavatorial
2 packages and 0 specfiles checked; 0 errors, 3 warnings.

Good.
Comment 2 Miro Hrončok 2019-01-20 12:05:35 UTC 
5. ...if it happens to stop being a directory.
Comment 3 Fabio Valentini 2019-01-20 12:27:14 UTC 
(In reply to Miro Hrončok from comment #1)
> 1. Note that there is also python2-backports-functools_lru_cache which IMHO
> makes more sense than using python2-repoze-lru. However both packages are
> still needed by stuff in Fedora so switching to one or another won't unblock
> the remaining one, so this is not important.

I didn't change this, it was introduced in the original python-jsonschema package.
If either of them are going to be removed, we can switch it to use the other one.
I don't care either way, but what's currently there is somewhat battle-tested.

> 2. Are the RHEL6 conditionals worth it? Will the package even build on
> RHEL6? Are we planning to change anything in EPEL6 about this? I hope we are
> not.

You're right, EPEL6 is definitely out of scope for this package, so I'll drop this.

> 3. The manual "Requires: python2-repoze-lru" is not needed, the patch
> patches setup.py and hence the package requires python2.7dist(repoze.lru).

Right.

> 4. Is %{_bindir}/jsonschema-2 needed? Can we not ship it and only consider
> this a compat library package?

I don't think anything depends on a python2 version of this script. I'll drop it.

> 5. I suggest to use
> %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info/ with the leading
> slash so no surprises happen if it happens to stop being a directory. 

Done.

> 6. setup.py uses setuptools, BR python2-setuptools explicitly (the
> transitive dependency can go away in the future, see python2.spec for
> details).

Done.

> Everything else looks good.
> 
> Note A: Depends on deprecated python2 but is a rename, so it is allowed.
> 
> Note B: It is a rename, but no binary package rename, so no new
> provides/obsoletes are needed.

I also bumped the Release, so it should be a clean upgrade over the existing python2 package from python-jsonschema.

> CHECKSUM(SHA256) this package     :
> 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02
> CHECKSUM(SHA256) upstream package :
> 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02
> 
> 
> Rpmlint
> -------
> Checking: python2-jsonschema-2.6.0-7.fc30.noarch.rpm
>           python2-jsonschema-2.6.0-7.fc30.src.rpm
> python2-jsonschema.noarch: W: spelling-error %description -l en_US validator
> -> lavatorial
> python2-jsonschema.noarch: W: no-manual-page-for-binary jsonschema-2
> python2-jsonschema.src: W: spelling-error %description -l en_US validator ->
> lavatorial
> 2 packages and 0 specfiles checked; 0 errors, 3 warnings.
> 
> Good.


I think I addressed all your comments.

Spec URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema.spec
SRPM URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema-2.6.0-8.fc29.src.rpm
Comment 4 Miro Hrončok 2019-01-20 13:11:00 UTC 
You have addressed all my comments. Package APPROVED.
Comment 5 Miro Hrončok 2019-01-20 13:17:49 UTC 
I suggest to have the python-jsonschema update ready before this is built in rawhide to hava two python2-jsonschemas for only a short period of time.
Comment 6 Fabio Valentini 2019-01-20 14:17:25 UTC 
Thanks! I've prepared the python-jsonschema update and sent off the PR.
I'll wait with building this package until everything is push-button ready.
Comment 7 Gwyn Ciesla 2019-01-22 14:36:49 UTC 
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python2-jsonschema
Comment 8 Fabio Valentini 2019-01-22 15:15:44 UTC 
I have successfully built this package for rawhide:
https://koji.fedoraproject.org/koji/taskinfo?taskID=32192528

Thanks everybody!
Note You need to log in before you can comment on or make changes to this bug.