Bug 509933 - Review Request: chameleon - Database schema transformation tool.
Review Request: chameleon - Database schema transformation tool.
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-06 19:03 EDT by Jeff Ortel
Modified: 2009-08-11 18:39 EDT (History)
2 users (show)

See Also:
Fixed In Version: 0.2-1.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-08-11 18:35:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jeff Ortel 2009-07-06 19:03:54 EDT
Spec URL: http://svn.fedorahosted.org/svn/chameleon/trunk/chameleon.spec
SRPM URL: https://fedorahosted.org/releases/c/h/chameleon/chameleon-0.1-6.fc10.src.rpm
Description: Chameleon is a tool used to transform database schema (DDL/SQL) files from a common DDL/SQL to database specific schema files.  Target users are projects that need to maintain schema creation/upgrade files and support multiple databases.  Initially supported databases are Oracle and PostgreSQL.
Comment 1 Jason Tibbitts 2009-07-10 21:55:57 EDT
I built this and noted that the python module isn't in the usual place for python modules, which ends up causing the files to not be byte-compiled.  This could cause problems if, for example, root runs the program and python decides to byte-compile the module then.  This used to be an issue, but I don't really know enough about python to know if it still is or what to do about it.
Comment 2 Jason Tibbitts 2009-07-10 22:28:02 EDT
OK, the problem was that python isn't in the build root.  Adding
  BuildRequires: python
fixes it.

You should also use %{_datadir} instead of hardcoding /usr/share and use cp -p to copy your files so the timestamps are preserved.
Comment 3 Jeff Ortel 2009-07-13 11:34:19 EDT
(In reply to comment #1)
> I built this and noted that the python module isn't in the usual place for
> python modules, which ends up causing the files to not be byte-compiled.  This
> could cause problems if, for example, root runs the program and python decides
> to byte-compile the module then.  This used to be an issue, but I don't really
> know enough about python to know if it still is or what to do about it. 

Hey Jason -- thanks for the review :)

I believe that installing chameleon in /usr/share/ is more appropriate then site-packages because chameleon is an application and not an extension to python libs (packages).
Comment 4 Jeff Ortel 2009-07-13 11:37:00 EDT
(In reply to comment #2)
> OK, the problem was that python isn't in the build root.  Adding
>   BuildRequires: python
> fixes it.
> 
> You should also use %{_datadir} instead of hardcoding /usr/share and use cp -p
> to copy your files so the timestamps are preserved.  

Great comments.  Thanks.

Applied in http://svn.fedorahosted.org/svn/chameleon/trunk/chameleon.spec and https://fedorahosted.org/releases/c/h/chameleon/chameleon-0.1-7.fc10.src.rpm.
Comment 5 Jeff Ortel 2009-07-13 11:46:24 EDT
For testing, the .sql files in http://svn.fedorahosted.org/svn/chameleon/trunk/samples can be used.

help:
  chameleon -h

try:

wget http://svn.fedorahosted.org/svn/chameleon/trunk/samples/sample.sql 
chameleon -s postgres sample.sql
chameleon -s oracle sample.sql

Planning on including a man page and documentation on the wiki more information in the README.
Comment 6 Jason Tibbitts 2009-08-05 14:11:30 EDT
Just went to review this, but the spec above doesn't match the src.rpm above.
Comment 7 Jeff Ortel 2009-08-05 14:33:20 EDT
(In reply to comment #6)
> Just went to review this, but the spec above doesn't match the src.rpm above.  

This: https://fedorahosted.org/releases/c/h/chameleon/chameleon-0.2-1.fc10.src.rpm matches.
Comment 8 Jason Tibbitts 2009-08-05 21:36:25 EDT
It's odd that rpm doesn't generate a python(abi) dependency for this package, but I guess that's because the modules aren't in the usual location for python modules.  I don't think that causes any problems, because as far as I know even the .pyc and .pyo files are compatible between python versions.

Otherwise, this builds fine and rpmlint is silent.  The web site is a little sparse but I assume someone will take care of that as the software is developed further.  I don't see any issues with the package.

* source files match upstream.  sha256sum:
   e70e8fa4cf67467f9ad90be0ee92479234e594306a638cc548c4150af96b189c
   chameleon-0.2.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 included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   chameleon = 0.2-1.fc12
  =
   /usr/bin/env
   python >= 2.3
   python-ply

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

APPROVED
Comment 9 Jeff Ortel 2009-08-06 10:54:41 EDT
Thanks for the review Jason :)

Since chameleon is an application (tool) rather then an extension of python libs, it seemed inappropriate to install in site-packages.  After review of several other python tools, including RHN, /usr/share seemed to be consistent with other projects.

As for the trac wiki, you're right, it's very sparse.  This tool is a build requirement for the Fedora Spacewalk project and I needed to get this out there ASAP.  I intend to provide a rich wiki, documentation and epydocs for the code in the coming weeks.
Comment 10 Jeff Ortel 2009-08-06 11:44:45 EDT
New Package CVS Request
=======================
Package Name: chameleon
Short Description: Database schema transformation tool.
Owners: jortel
Branches: F-10 F-11 EL-4 EL-5
InitialCC: jortel
Comment 11 Kevin Fenzi 2009-08-07 16:05:06 EDT
cvs done.
Comment 12 Fedora Update System 2009-08-10 12:49:52 EDT
chameleon-0.2-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/chameleon-0.2-1.fc11
Comment 13 Fedora Update System 2009-08-10 12:49:57 EDT
chameleon-0.2-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/chameleon-0.2-1.fc10
Comment 14 Fedora Update System 2009-08-11 18:35:04 EDT
chameleon-0.2-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2009-08-11 18:39:26 EDT
chameleon-0.2-1.fc10 has been pushed to the Fedora 10 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.