Bug 452388 - Review Request: python-migrate - Schema migration tools for SQLAlchemy
Review Request: python-migrate - Schema migration tools for SQLAlchemy
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Luke Macken
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-21 16:24 EDT by Toshio Ernie Kuratomi
Modified: 2008-07-23 22:13 EDT (History)
4 users (show)

See Also:
Fixed In Version: 0.4.4-4.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-17 17:16:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
lmacken: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Toshio Ernie Kuratomi 2008-06-21 16:24:29 EDT
Spec URL: http://toshio.fedorapeople.org/packages/python-migrate.spec
SRPM URL: http://toshio.fedorapeople.org/packages/python-migrate-0.4.4-1.src.rpm

Description:
Schema migration tools for SQLAlchemy designed to support an agile approach
to database design and make it easier to keep development and production
databases in sync as schema changes are required.  It allows you to manage.
database change sets and database repository versioning.
Comment 1 Toshio Ernie Kuratomi 2008-06-21 16:26:01 EDT
Ricky and I are working on this together.  Reviewers, it's fine with me if he
works on modifying the package to finish the review as I'm going on vacation for
a bit more than a week.
Comment 2 Jason Tibbitts 2008-06-21 17:37:43 EDT
Hmm, 404 on that URL.  I used
http://toshio.fedorapeople.org/packages/python-migrate-0.4.4-1.fc9.src.rpm instead.

Unfortunately it doesn't build in mock on rawhide:

+ /usr/bin/python setup.py build
Traceback (most recent call last):
  File "setup.py", line 2, in <module>
    from setuptools import setup,find_packages
ImportError: No module named setuptools

I added a python-setuptools dependency and it builds OK.  With that added it
builds OK.  Full review forthcoming....
Comment 3 Jason Tibbitts 2008-06-21 18:27:54 EDT
Aside from the build error, which I'll just assume is fixed in order to progress
with this review, I note that /usr/bin/migrate is just begging for conflicts. 
While no package in Fedora provides it, a quick search shows that it's used in
at least one clustering environment, an HSM system, something Zope related, and
some user migration tools.  Is there any other reasonable choice of name that
could be used?

I note that there's a test suite in the tarball; it looks like it should be
possible to run this with sqlite, but I have no idea how or if it's at all
possible to do that at runtime.

A minor nit, I guess, but you might as well remove the unneeded comments left
over from the specfile template.

* source files match upstream:
  2877b1b95c0e34aabc61c81636a136bc2af900703aa580bc4321bab65f48411a  
   sqlalchemy-migrate-0.4.4.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
X BuildRequires missing python-setuptools (at least in rawhide)
* %clean is present.
* package builds in mock (after adding python-setuptools dependency).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   python-migrate = 0.4.4-1.fc10
  =
   /usr/bin/python
   python(abi) = 2.5

? %check is not present, but there's a test suite,
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 4 Ricky Zhou 2008-07-06 21:01:10 EDT
I wasn't sure how to handle the possible /usr/bin/migrate conflict, but here's
an updated spec with some updated Requires and BuildRequires:

Spec URL: http://ricky.fedorapeople.org/pkgs/python-migrate/python-migrate.spec
SRPM URL:
http://ricky.fedorapeople.org/pkgs/python-migrate/python-migrate-0.4.4-2.fc9.src.rpm

I did look into the %check thing a bit, and it looks like the tests require the
py library (http://codespeak.net/py/dist/) which isn't in Fedora yet.  Once py
is in,
http://ricky.fedorapeople.org/pkgs/python-migrate/python-migrate.tests.spec
should work (python setup.py test gave errors - it looks like py.test doesn't
have any classes to work with setuptools).  Overall, py wasn't that much fun to
work with, and I'm not sure if it was worth the hackyness in the specfile with
the BuildRequires on python-sqlite2 and everything.  
Comment 5 Toshio Kuratomi 2008-07-07 13:12:22 EDT
I queried upstream about the name.  They aren't too worried about potential
conflicts (their reasoning is Debian has imported it with /usr/bin/migrate).  I
asked and they did say that sqlalchemy-migrate seemed like a good name if we
wanted to do that locally.

So I'm in favour of renaming migrate => sqlalchemy-migrate.

I looked at py.test as well but it wasn't trivial to package.  I could probably
get it working with some more work but I'm still swamped with post-vacation work
so I concur with holding off on that until py.test is included.
Comment 6 Toshio Ernie Kuratomi 2008-07-15 21:31:37 EDT
Okay, new package ready for review.

http://toshio.fedorapeople.org/packages/python-migrate-0.4.4-3.fc9.src.rpm
http://toshio.fedorapeople.org/packages/python-migrate.spec

* Binary renamed to /usr/bin/sqlalchemy-migrate
* Check section present but commented out.  If we get py.test in we can look at
enabling it.
* BuildRequires picked up from ricky's update
* Extraneous comments removed

tibbs: I know you're going on vacation.  I think lmacken might have time to look
at this so you can enjoy yourself :-)
Comment 7 Luke Macken 2008-07-16 16:23:13 EDT
With regard to the naming issue, I'm also in favor of s/migrate/sqlalchemy-migrate/.

? rpmlint complaint: python-migrate.noarch: E: non-executable-script
/usr/lib/python2.5/site-packages/migrate/versioning/templates/manage.py_tmpl 0644
* Package name is OK, and I'd be fine with the s/migrate/sqlalchemy-migrate/
rename as well.
* Package meets the packaging guidelines
* License is valid and meets guidelines
* Specfile is clean
* Source file matches upstream
* Package successfully builds in mock
* Requires and BuildRequires are correct
* Package owns everything it creates
* Does not contain duplicate files
* Permissions are set properly
* %clean section exists
* consistent use of macros
* code, not content
* small amounts of documentation, no subpackage
* %docs do not effect runtime
* buildroot is wiped before installation
* filenames are all valid utf-8

I don't see the rpmlint "error" as an issue, since it is a template that turns
into a script once installed.

So, the rename is up to you guys -- whether we want python-migrate or
python-sqlalchemy-migrate (I tend to lean towards the latter).  Other than that...

Approved!
Comment 8 Toshio Ernie Kuratomi 2008-07-16 18:48:58 EDT
Cool.  The package naming is a bit of a pain:

If we think of this as primarily a python module with a program attached to it
then it should be: python-migrate to follow the naming guidelines for python
modules (import migrate => python-migrate)

If we think of it as a program with most functionality implemented in a module,
then it could be named migrate or sqlalchemy-migrate.

debian has python-migrate.  Gentoo doesn't seem to have it packaged yet.  So I
think I'll leave it at python-migrate for now.
Comment 9 Toshio Ernie Kuratomi 2008-07-16 18:51:14 EDT
New Package CVS Request
=======================
Package Name: python-migrate
Short Description: Schema migration tools for SQLAlchemy
Owners: toshio lmacken ricky
Branches: F-8 F-9 devel EL-5 
InitialCC:
Cvsextras Commits: yes
Comment 10 Kevin Fenzi 2008-07-16 20:17:26 EDT
cvs done.
Comment 11 Fedora Update System 2008-07-17 17:13:25 EDT
python-migrate-0.4.4-4.fc8 has been submitted as an update for Fedora 8
Comment 12 Fedora Update System 2008-07-17 17:14:49 EDT
python-migrate-0.4.4-4.fc9 has been submitted as an update for Fedora 9
Comment 13 Toshio Ernie Kuratomi 2008-07-17 17:16:43 EDT
Thanks all!

Built in devel.  Will be pushed to F8 and F9.
Comment 14 Fedora Update System 2008-07-23 22:12:11 EDT
python-migrate-0.4.4-4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2008-07-23 22:13:05 EDT
python-migrate-0.4.4-4.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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