Bug 701426 - Review Request: python-taboot - Client utility for scripted multi-system administration over Func
Summary: Review Request: python-taboot - Client utility for scripted multi-system admi...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 702892
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-05-02 20:17 UTC by Tim Bielawa
Modified: 2011-06-02 19:24 UTC (History)
4 users (show)

Fixed In Version: python-taboot-0.2.13-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-05-12 20:29:15 UTC
kevin: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Tim Bielawa 2011-05-02 20:17:10 UTC
Spec URL: http://people.redhat.com/~tbielawa/taboot/python-taboot.spec
SRPM URL: http://people.redhat.com/~tbielawa/taboot/releases/taboot-latest/python-taboot-0.2.11-1.fc14.src.rpm
Description: 
Tool for managing and executing tasks related to software releases and
system administration via customizable YAML scripts. A library of
prewritten tasks is built in so you can get started immediately.

Taboot uses Func for inter-system communication, eliminating hacky ssh
commands and the overhead associated with key management. Taboot can
be extended by writing your Func modules and tasks in Python.


This is my first package here so I will need a sponsor. Thanks!

Comment 1 Ken Dreyer 2011-05-04 11:32:18 UTC
Welcome Tim! Please block FE-NEEDSPONSOR. http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Comment 2 Tim Bielawa 2011-05-04 13:47:22 UTC
Oh man, missed that! I thought I had gotten everything.

It's blocking now. Thanks for the pointer.

Comment 3 Kevin Fenzi 2011-05-04 16:13:05 UTC
I'm not sure when I might get to it, but if no one beats me to it I am interested in reviewing this and sponsoring you. ;) (Other sponsors feel free to take it before then. :)

Comment 4 Tim Bielawa 2011-05-05 15:41:01 UTC
Today I released Taboot 0.2.12-1. The specfile listed has been updated to reflect this.

The new SRPM address is:

http://people.redhat.com/~tbielawa/taboot/releases/taboot-latest/python-taboot-0.2.12-1.fc14.src.rpm

Side note: I make concurrent releases for el5/el6/FC14. There are SRPMs for el5 and el6 in that directory as well.

Comment 5 Kevin Fenzi 2011-05-07 19:24:46 UTC
ok, I am going to try and get this reviewed this weekend. ;) 

Look for a review in a while... 

Do you have any other packages you are going to submit at this time?

Comment 6 Kevin Fenzi 2011-05-07 19:45:00 UTC
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License GPLv3+
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
676c7ef0093bbd43298cedf934420143  python-taboot-0.2.12.tar.gz
676c7ef0093bbd43298cedf934420143  python-taboot-0.2.12.tar.gz.orig
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
See below - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. Not a blocker, but: There's no need to use macros for things that are longer than 
the command they replace, ie, %{__make} vs just 'make'. I think the non macro versions
make the spec more readable, but it's up to you. 

2. Both the main package and subpackage require func, so what is the advantage of 
having the subpackage? I guess to install on clients only? 

3. You don't need a clean section if you aren't targeting EPEL (which I hope you are)
but why the "%{__make} clean" at the top of it? 

4. rpmlint says: 
taboot-func.noarch: W: no-documentation
which can be ignored.

Comment 7 Tim Bielawa 2011-05-08 03:22:00 UTC
Kevin, thanks for taking the time to review this!

> Issues: 
> 
> 1. Not a blocker, but: There's no need to use macros for things that are longer...

You just cleared something up I've been wondering for a while. I figured the macros were for compatibility across build targets. But yeah you're right, cp and rm are probably not going to have any compatibility issues.

I'm going to leave them in though since: 1) they're already there, and 2) the uniformity makes the %install section look nice and readable.


> 2. Both the main package and subpackage require func, so what is the advantage...

- python-taboot is a client package that you can install on any minion you run commands from,  your 'command-center' you might say.

- taboot-func is a func module that gets installed on target machines. I'll update the %description of that, its purpose could be much clearer.

Specifically: taboot-func provides a func interface to the mod_jk API on tomcat JK balancers. The way we use Taboot now we have 'command center' type host in a given environment that is granted access to the other minions. From there the func command would go to a machine and utilize the modjk func module that was installed by taboot-func.


> 3. You don't need a clean section if you aren't targeting EPEL (which I hope you are) but why 
> the "%{__make} clean" at the top of it? 

That is a very good question. I'll see it's unnecessary and remove it if so.

Yes, I do intend to target EPEL. From the EPEL Package Maintainers Page [0] I see it says that after this is approved for Fedora I can then go on to request EPEL branches. What does that require of me exactly?


> Do you have any other packages you are going to submit at this time?

When you asked about taboot-func it reminded me that the modjkapi library needs to be available too. It's already packaged with a specfile, I just need to build it and clean out any rpmlint that might show up.



I'll reply to this ticket again as soon as I have another release addressing the issues you brought up. Also included will be a reference to the modjkapi library review request.


[0] http://fedoraproject.org/wiki/EPEL_Package_Maintainers

Comment 8 Tim Bielawa 2011-05-08 05:33:38 UTC
> 2. Both the main package and subpackage require func, so what is
> the advantage...

Description of taboot-func has been updated in this release.

> 3. You don't need a clean section if you aren't targeting EPEL (which I hope
> you are) but why the "%{__make} clean" at the top of it? 

I removed "make clean" and the world didn't come to an end. It's gone in this release too.

> Do you have any other packages you are going to submit at this time?

Now blocking this ticket is a review request for python-modjkapi. taboot-func has been updated to depend on it (it should have been doing that before now anyway).


Specfile url is the same: http://people.redhat.com/~tbielawa/taboot/python-taboot.spec

SRPM is now: http://people.redhat.com/~tbielawa/taboot/releases/taboot-0.2.13/python-taboot-0.2.13-1.fc14.src.rpm

Thanks for your time Kevin

Comment 9 Kevin Fenzi 2011-05-11 19:44:33 UTC
I don't see any further blockers here, so this package is APPROVED.

Comment 10 Tim Bielawa 2011-05-12 01:49:51 UTC
New Package SCM Request
=======================
Package Name: python-taboot
Short Description: Client utility for scripted multi-system administration over Func
Owners: tbielawa
Branches: f14 f15 el5 el6
InitialCC:

Comment 11 Jason Tibbitts 2011-05-12 16:40:33 UTC
Git done (by process-git-requests).

Comment 12 Tim Bielawa 2011-05-12 20:29:15 UTC
Koji builds complete:

 - el5
 - el6
 - f14

Comment 13 Fedora Update System 2011-05-12 20:35:27 UTC
python-taboot-0.2.13-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/python-taboot-0.2.13-1.el5

Comment 14 Fedora Update System 2011-05-12 20:37:16 UTC
python-taboot-0.2.13-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/python-taboot-0.2.13-1.el6

Comment 15 Fedora Update System 2011-05-12 20:54:38 UTC
python-taboot-0.2.13-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/python-taboot-0.2.13-1.fc14

Comment 16 Fedora Update System 2011-05-25 02:28:55 UTC
python-taboot-0.2.13-1.fc14 has been pushed to the Fedora 14 stable repository.

Comment 17 Fedora Update System 2011-06-02 19:03:26 UTC
python-taboot-0.2.13-1.el5 has been pushed to the Fedora EPEL 5 stable repository.

Comment 18 Fedora Update System 2011-06-02 19:24:22 UTC
python-taboot-0.2.13-1.el6 has been pushed to the Fedora EPEL 6 stable repository.


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