Bug 639874 (python-rocket) - Review Request: python-rocket - Modern, multi-threaded, comet-friendly WSGI web server
Summary: Review Request: python-rocket - Modern, multi-threaded, comet-friendly WSGI w...
Keywords:
Status: CLOSED WONTFIX
Alias: python-rocket
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-10-04 09:13 UTC by Ilia Cheishvili
Modified: 2014-04-17 00:27 UTC (History)
7 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-04-17 00:27:02 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ilia Cheishvili 2010-10-04 09:13:51 UTC
Spec URL: http://github.com/icheishvili/rpms/raw/master//python-rocket.spec
SRPM URL: http://github.com/icheishvili/rpms/raw/master//python-rocket-1.1.1-1.fc13.src.rpm
Description: This is a modern, multi-threaded, comet-friendly WSGI web server for python.  I am hoping to get it included into Extras.

The Rocket web server is a server designed to handle the increased needs of modern web applications implemented in pure Python. It can serve WSGI applications and static files. Rocket has the ability to be extended to handle different types of networked request-response jobs. Rocket runs on cPython 2.5- 3.x and Jython 2.5 (without the need to run through the 2to3 translation tool). Rocket is similar in purpose to Cherrypy's Wsgiserver but with added flexibility, speed and concurrency.

Comment 1 Thomas Spura 2010-10-20 11:45:31 UTC
(Didn't look much to this package...)

Why don't you build a python3 subpackage, if it's possible?
At least your description says so...

see:
https://fedoraproject.org/wiki/Packaging:Python

Comment 2 Ilia Cheishvili 2010-11-22 04:56:14 UTC
A very good point.  I will see about doing this.  Thanks for the wiki link.

Comment 3 Jeffrey Ness 2011-01-10 20:11:27 UTC
Greetings,

A few things I noticed while looking at your SPEC and Bug Review:
 
  * Every Review should include a rpmlint of both source rpm and rpm binary:
      http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

  * Add %{version} to your Source0 tag, this will allow easy version upgrades.
      http://pypi.python.org/packages/source/r/rocket/Rocket-%{version}.zip

Comment 4 Ilia Cheishvili 2011-01-11 04:58:41 UTC
I made all updates suggested:

Spec URL: http://github.com/icheishvili/rpms/raw/master/python-rocket.spec
SRPM URL:
http://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.2-1.fc14.src.rpm

Also, here's rpmlint output:

0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 5 Jeffrey Ness 2011-01-25 20:55:08 UTC
Here is a bit more detailed review in order to help get the package approved, as mentioned above this is unofficial.

Good:
*rpmlint clean
* Package follows naming guidelines
* Spec file name matches package name
* License is MIT in source and spec file
* MIT is an open source license
* Spec file is legible American English
* Source matches upstream: MD5sum b0bfa3bca9d30838c5dec4a083fbd247
* Builds in mock
* All build deps satisfied but see below; there's some extra ones.
* No locale files that need to be marked with %find_lang
* No shared libraries
* No bundled libraries
* Package is not relocatable
* No files listed more than once
* All files and directories created by the package owned by the package and no
others.
* Package contains code, not content.
* No large documentation that needs to be in a separate subpackage
* Nothing in %doc used at runtime
* No GUI application included so no .desktop requirement
* All filenames are valid utf-8
* No scriptlets
* No file dependencies
* No programs so no need for man pages

Needswork:
*You should use %global rather than %global
          http://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define#current_usage_of_.25define_beneath_Packaging

Cosmetic:
* Speak with upstream about adding LICENSE to source:
         http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text

Comment 6 Toshio Ernie Kuratomi 2011-01-25 21:26:35 UTC
Note: In your Requires: section you have an:
%if 0%{?with_python3}
BuildRequires:  python3-setuptools
BuildRequires:  python3-devel
Requires: python3
%else
Requires: python
%endif

You actually want to change that a little bit.  The "Requires: python" should be outside of the conditional since you want it to be a Requirement of the main python-rocket package whether or not a python3 subpackage is built.  The Requires: python3 portion should be part of the python3-rocket subpackage further down in the spec file so that only the subpackage has a dependency on python3.

BuildRequires stay at the top because one environment is setup for building all of the subpackages.  So there's no need to parcel those out to each subpackage section.

Comment 7 Ilia Cheishvili 2011-01-31 19:01:56 UTC
Jeffrey and Toshio, thanks for the reviews.  I have made the suggested changes.  Find the new SPEC and SRPM here:

https://github.com/icheishvili/rpms/raw/master/python-rocket.spec
https://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.3-1.fc14.src.rpm

Comment 8 Haïkel Guémar 2012-01-23 21:23:28 UTC
Are you still working on it ? 
* upstream has released rocket 1.2.4 since
* you should request upstream to add a License file
* are you planning to maintain this package on EPEL5 ? if not, you should remove any reference to buildroot cleaning and defattr
* you should keep updating the changelog, at least when updating upstream releases (some reviewers might indulge that you don't bump release field but nobody will approve this package if you don't maintain properly your changelog)

Comment 9 Ilia Cheishvili 2012-02-27 04:02:30 UTC
Thanks for pointing this out.  Here are the updated versions:

https://github.com/icheishvili/rpms/raw/master/python-rocket.spec
https://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.4-1.fc16.src.rpm

I do have a question for you, however.  What do you mean when you say I should take out "any reference to buildroot cleaning and defattr" unless I intend to maintain the package in epel5?  Perhaps I'm just not understanding how the two relate to each other.

I have also asked upstream to include a LICENSE file: https://answers.launchpad.net/rocket/+question/188972

Comment 10 Haïkel Guémar 2012-02-27 06:54:33 UTC
1. %defattr is no more required since RPM 4.4, it is implied since.
2. The Buildroot tag is ignored, the provided buildroot will be automatically cleaned (since F10).

That is valid for all currently supported Fedora branches and EPEL6 branch but not EPEL5.
According packaging guidelines, these must be removed unless you plan to maintain your package on EPEL5 (for which you're granted an exception).

Comment 12 Mario Blättermann 2012-10-07 20:46:57 UTC
Your package is almost fine now, but the source tarball contains a bundled egg-info. You have to remove it before building your package (in the %prep section):

rm -rf %{upstream_name}.egg-info

See https://fedoraproject.org/wiki/Packaging:Python_Eggs#Upstream_Eggs for more information.


Taking this for a full review.

Comment 13 Mario Blättermann 2012-10-07 20:51:30 UTC
Another issue: You were forgotten to bump the release tag after your last change. Either bump it or concatenate the changelog entries to a single one. Doesn't matter in the pre-Git state of your package.

Comment 14 Mario Blättermann 2012-10-28 16:55:01 UTC
Ping...?

Comment 15 Mario Blättermann 2012-12-12 14:01:57 UTC
Ping again...?

Comment 17 Mario Blättermann 2013-01-17 20:27:13 UTC
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4879379

$ rpmlint -i -v *
python3-rocket.noarch: I: checking
python3-rocket.noarch: W: spelling-error Summary(en_US) multi -> mulch, mufti
The value of this tag appears to be misspelled. Please double-check.

python3-rocket.noarch: W: spelling-error %description -l en_US cPython -> c Python, Python, python
The value of this tag appears to be misspelled. Please double-check.

python3-rocket.noarch: I: checking-url http://pypi.python.org/pypi/rocket/ (timeout 10 seconds)
python3-rocket.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

python-rocket.src: I: checking
python-rocket.src: W: spelling-error Summary(en_US) multi -> mulch, mufti
The value of this tag appears to be misspelled. Please double-check.

python-rocket.src: W: spelling-error %description -l en_US cPython -> c Python, Python, python
The value of this tag appears to be misspelled. Please double-check.

python-rocket.src: I: checking-url http://pypi.python.org/pypi/rocket/ (timeout 10 seconds)
python-rocket.src: I: checking-url http://pypi.python.org/packages/source/r/rocket/Rocket-1.2.4.zip (timeout 10 seconds)
python-rocket.noarch: I: checking
python-rocket.noarch: W: spelling-error Summary(en_US) multi -> mulch, mufti
The value of this tag appears to be misspelled. Please double-check.

python-rocket.noarch: W: spelling-error %description -l en_US cPython -> c Python, Python, python
The value of this tag appears to be misspelled. Please double-check.

python-rocket.noarch: I: checking-url http://pypi.python.org/pypi/rocket/ (timeout 10 seconds)
python-rocket.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

python-rocket.spec: I: checking-url http://pypi.python.org/packages/source/r/rocket/Rocket-1.2.4.zip (timeout 10 seconds)
3 packages and 1 specfiles checked; 0 errors, 8 warnings.


Some ignorable spelling errors, nothing of interest.



There remain still some issues:

If your package isn't been targeted for EPEL5, please drop the following parts of your spec.

rm -rf %{buildroot} (in %install)

%clean
rm -rf %{buildroot}


And drop in any case:

CFLAGS="$RPM_OPT_FLAGS" (it's a noarch package)

Comment 18 Christopher Meng 2014-02-23 16:05:03 UTC
ping again.

Do you want to finish this still?

Comment 19 Ilia Cheishvili 2014-04-17 00:27:02 UTC
I no longer have a need for it and it is not actively maintained upstream anymore. So I do not want to finish packaging this.


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