https://bugzilla.redhat.com/show_bug.cgi?id=1389404
Bug ID: 1389404 Summary: Review Request: python-Flask-PyMongo - PyMongo support for Flask applications Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: david.hannequin@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-Flask-PyMon... SRPM URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-Flask-PyMon...
Description: PyMongo support for Flask applications. Fedora Account System Username: hvad
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |athoscribeiro@gmail.com Assignee|nobody@fedoraproject.org |athoscribeiro@gmail.com Flags| |fedora-review?
--- Comment #1 from Athos Ribeiro athoscribeiro@gmail.com --- I am taking this.
1st of all, do you have any comments on having the package to own /usr/lib/pythonX/site-packages/tests directory and having files in that directory?
I mean, what if there was a package named python-tests? Also, what if another package goes on with the same decision and wants to own, let's say /usr/lib/python3.5/site-packages/tests/util.py ?
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
--- Comment #2 from David Hannequin david.hannequin@gmail.com --- Hi,
You right directory /usr/lib/pythonX/site-packages/tests isn't necessary for package.
I delete this directory after test rpm.
Best regard
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
--- Comment #3 from Athos Ribeiro athoscribeiro@gmail.com --- Hi David,
Naming:
- All the Flask related packages are named with lower case. Did you consider that when naming this package?
License:
- The license file you are using belongs to the "Flask Sphinx Style" used in the documentation, not to this package.
- Note that since this package is shipped under a BSD license, it must contain the full license text. The text is present in both files shipped in the package (__init__.py and wrappers.py), but it would be really nice if you considered asking upstream to also include the text in a separate file (like LICENSE or COPYING). This is not a blocker for this review, since the text is already available in the files I mentioned.
Tests:
- Upstream provides a test suite. Is there any reason for not running the tests in %check?
Documentation:
- There are some import errors being raised while building documentation, did you check that?
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
--- Comment #4 from David Hannequin david.hannequin@gmail.com --- Hi Athos,
I fix naming and add tests :
Spec URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-flask-pymon...
SRPM URL: https://hvad.fedorapeople.org/fedora/python-Flask-PyMongo/python-flask-pymon...
I'll contact upstream. Could it pass anyway ?
Best regard
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
--- Comment #5 from Athos Ribeiro athoscribeiro@gmail.com --- Hi David,
- Upstream already ships a LICENSE file [1], you should use it (no need to contact upstream).
- For the python2- subpackage, you want to use the python2-* Requires and BR, not just python-* (they are already available for your dependencies).
- The licence file you are shipping in python2 and python3 subpackages belongs to the documentation theme: it should go in the documentation subpackage instead, not in the binary packages.
- You are removing the tests directory in %build, so there's no tests to run in %check.
- There are (still) some import errors being raised while building documentation, did you check that [2]?
[1] https://github.com/dcrosta/flask-pymongo/blob/master/LICENSE
[2]
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.tEhIGH ... + sphinx-build docs html
... looking for now-outdated files... /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:59: WARNING: autodoc: failed to import method u'Collection.find_one_or_404' from module u'flask_pymongo.wrappers'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:61: WARNING: autodoc: failed to import method u'PyMongo.send_file' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:63: WARNING: autodoc: failed to import method u'PyMongo.save_file' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:65: WARNING: autodoc: failed to import class u'BSONObjectIdConverter' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:193: WARNING: autodoc: failed to import data u'ASCENDING' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:195: WARNING: autodoc: failed to import data u'DESCENDING' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:201: WARNING: autodoc: failed to import class u'PyMongo' from module u'flask_pymongo'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:204: WARNING: autodoc: failed to import class u'Collection' from module u'flask_pymongo.wrappers'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:217: WARNING: autodoc: failed to import class u'MongoClient' from module u'flask_pymongo.wrappers'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:220: WARNING: autodoc: failed to import class u'MongoReplicaSetClient' from module u'flask_pymongo.wrappers'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors /builddir/build/BUILD/Flask-PyMongo-0.5.1/docs/index.rst:223: WARNING: autodoc: failed to import class u'Database' from module u'flask_pymongo.wrappers'; the following exception was raised: Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 551, in import_object __import__(self.modname) File "/builddir/build/BUILD/Flask-PyMongo-0.5.1/flask_pymongo/__init__.py", line 29, in <module> from bson.errors import InvalidId ImportError: No module named bson.errors none found
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
David Hannequin david.hannequin@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |python-Flask-PyMongo - |python-flask-pymongo - |PyMongo support for Flask |PyMongo support for Flask |applications |applications
https://bugzilla.redhat.com/show_bug.cgi?id=1389404
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #6 from Robert-André Mauchin zebob.m@gmail.com --- - Use BuildRequires: python3-sphinx, not python-sphinx
- Download the LICENSE file and install it:
Source1: https://raw.githubusercontent.com/dcrosta/flask-pymongo/master/LICENSE
And:
%prep %autosetup -n %{pypi_name}-%{version} rm -rf %{pypi_name}.egg-info
cp %{S:1} .
Then install it in %files:
%files -n python2-flask-pymongo %license LICENSE %doc README.md %{python2_sitelib}/flask_pymongo %{python2_sitelib}/Flask_PyMongo-%{version}-py*egg-info
%files -n python3-flask-pymongo %license LICENSE %doc README.md %{python3_sitelib}/flask_pymongo %{python3_sitelib}/Flask_PyMongo-%{version}-py*egg-info
%files -n python-flask-pymongo-doc %license docs/_themes/LICENSE %doc html
- Bump to 0.5.2
- Generate the docs with:
# generate html docs PYTHONPATH=${PWD} sphinx-build-3 docs html # remove the sphinx-build leftovers rm -rf html/.{doctrees,buildinfo}
- Add the required BR to generate the docs:
BuildRequires: python2dist(flask) >= 0.8 BuildRequires: python2dist(pymongo) >= 2.5 BuildRequires: python2-pymongo-gridfs
BuildRequires: python3dist(flask) >= 0.8 BuildRequires: python3dist(pymongo) >= 2.5 BuildRequires: python3-pymongo-gridfs
- Not useful:
rm -rf %{python2_sitelib}/tests
- The tests are not included with the Pypi anchive, remove:
%check %{__python2} setup.py test %{__python3} setup.py test
- You could use the new python2dist/python3dist macros:
BuildRequires: python2-devel BuildRequires: python2dist(setuptools) BuildRequires: python2dist(flask) >= 0.8 BuildRequires: python2dist(pymongo) >= 2.5 BuildRequires: python2-pymongo-gridfs
BuildRequires: python3-devel BuildRequires: python3dist(setuptools) BuildRequires: python3dist(sphinx) BuildRequires: python3dist(flask) >= 0.8 BuildRequires: python3dist(pymongo) >= 2.5 BuildRequires: python3-pymongo-gridfs
And:
%package -n python2-flask-pymongo Summary: %{summary} %{?python_provide:%python_provide python2-flask-pymongo}
Requires: python2dist(flask) >= 0.8 Requires: python2dist(pymongo) >= 2.5
And:
%package -n python3-flask-pymongo Summary: %{summary} %{?python_provide:%python_provide python3-flask-pymongo}
Requires: python3dist(flask) >= 0.8 Requires: python3dist(pymongo) >= 2.5
package-review@lists.fedoraproject.org