[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Haïkel Guémar  changed:

   What|Removed |Added

 Status|ON_QA   |CLOSED
 Resolution|--- |CURRENTRELEASE
Last Closed||2015-11-18 08:34:49



-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Alan Pevec  changed:

   What|Removed |Added

 Status|ASSIGNED|POST
 CC||ape...@redhat.com



--- Comment #14 from Alan Pevec  ---
Imported to rdo-liberty distgit
https://github.com/openstack-packages/aodh/commits/rdo-liberty

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Alan Pevec  changed:

   What|Removed |Added

 Status|MODIFIED|ON_QA



--- Comment #16 from Alan Pevec  ---
In cloud7-openstack-liberty-testing

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Alan Pevec  changed:

   What|Removed |Added

 Status|POST|MODIFIED
   Fixed In Version||openstack-aodh-1.0.0-1.el7



--- Comment #15 from Alan Pevec  ---
http://cbs.centos.org/koji/buildinfo?buildID=6878

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #13 from Pradeep Kilambi  ---
Thanks! fixed the /etc/aodh own issue.

Spec URL: https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh.spec
SRPM URL:
https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh-1.0.0-1.fc22.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Haïkel Guémar  changed:

   What|Removed |Added

 Target Release|--- |trunk
  Component|Package Review  |Package Review
Version|rawhide |trunk
Product|Fedora  |RDO
   Target Milestone|--- |GA
 QA Contact|extras...@fedoraproject.org |hgue...@redhat.com
  Flags|fedora-review?  |



-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #10 from hgue...@redhat.com ---
Hi Prad, I was able to run aodh services now with fixed packages, still some
changes needed:
* %{__python} to %{__python2} (by default, the former will be /usr/bin/python3
in Fedora)
* You should move %license LICENSE to python-aodh. If you install python-aodh,
it doesn't install license file and it's pulled by common which itself is
pulled by all the other subpackages.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #11 from Pradeep Kilambi  ---
Thanks Haikel!

Addressed the above comments:

Spec URL: https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh.spec
SRPM URL:
https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh-1.0.0-1.fc22.src.rpm

Please review.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550
Bug 1243550 depends on bug 1233168, which changed state.

Bug 1233168 Summary: Review Request: python-oslo-service - Oslo service library
https://bugzilla.redhat.com/show_bug.cgi?id=1233168

   What|Removed |Added

 Status|ASSIGNED|CLOSED
 Resolution|--- |RAWHIDE



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Haïkel Guémar  changed:

   What|Removed |Added

 Depends On||1233168



--- Comment #7 from Haïkel Guémar  ---
* Latest tarball was not uploaded in PyPI, could you get it fixed?
If we can't, we could switch to launchpad url but they have OpenStack release
name there.
* License should be ASL 2.0
* We have to fix oslo.messaging first (needs oslo.service first) => I'm
currently fixing it.

Minor stuff:
* wrapping openstack-aodh-compat description (line too long)
* cleanup tabs from spec files


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1233168
[Bug 1233168] Review Request: python-oslo-service - Oslo service library
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #8 from Pradeep Kilambi  ---
(In reply to Haïkel Guémar from comment #7)
> * Latest tarball was not uploaded in PyPI, could you get it fixed?
> If we can't, we could switch to launchpad url but they have OpenStack
> release name there.


This is already in progress. The upstream pypi upload job was missing. The
pending review is here: https://review.openstack.org/#/c/222579/


> * License should be ASL 2.0


Sure, will fix that.


> * We have to fix oslo.messaging first (needs oslo.service first) => I'm
> currently fixing it.
> 
> Minor stuff:
> * wrapping openstack-aodh-compat description (line too long)
> * cleanup tabs from spec files

Will clean these up.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #9 from Pradeep Kilambi  ---
Addressed the above comments:

Spec URL: https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh.spec
SRPM URL:
https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh-1.0.0-1.fc22.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #6 from Pradeep Kilambi  ---
Thanks! addressed your comments, New spec and src are here:

Spec URL: https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh.spec
SRPM URL:
https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh-1.0.0-1.fc22.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #5 from Haïkel Guémar  ---
> That is correct. We discussed weather to keep these together or separate.
> The reason i split is because these services can be run on thier own nodes.
> Which would be hard to do if its a single package? If the recommendation is
> to keep these together as part of deprecation then i understand and can
> change it. 
> 

Thanks, I appreciate that you made it a bit clearer for me. :)
+1 for splitting the services in separate packages.


> 
> Yep understand, reason i dint add it yet is i was not sure which version to
> put in. Since liberty is not out yet? Ideally this version should be the
> final version of ceilometer-alarm package? Is it ok to add the current
> version and update later?
> 

Workaround would using a n-v-r like EPOCH:2015.1.99-1 that will be never
attained by kilo point-release.


> 
> I was thinking i can make the aodh-common obsolete ceilometer-alarm. Wouldnt
> that work? since aodh-common will be required by all sub-packages?
> 

When you'll update existing deployments aodh-common will be indeed installed
but won't pull aodh-notifier/evaluator in turn.
Let's have a dummy -compat subpackage that does the obsoletions and requires
the services.
The good thing, upgrades will work smoothly and new deployments won't be
affected an benefit from the split.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Haïkel Guémar  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 CC||pkila...@redhat.com
 Blocks||1243533
   ||(RDO-LIBERTY-REVIEWS)
  Flags||needinfo?(pkilambi@redhat.c
   ||om)



--- Comment #3 from Haïkel Guémar  ---
## Blockers


* I'll need your help to properly understand ceilometer/aodh relationship and
figure out the proper upgrade path.

I pretty much get that openstack-aodh-{notifier,evaluator} services replace
services with the same in ceilometer.
Problem is that openstack-ceilometer-{notifier,evaluator} were shipped in a
single openstack-ceilometer-alarm package, previously.

=> You already obsoletes openstack-ceilometer-alarm but it's unversioned,
should be:
Provides: openstack-ceilometer-alarm = 2:%{version}-%{release}
Obsoletes: openstack-ceilometer-alarm < last n-v-r with epoch of
openstack-ceilometer-alarm

As ceilometer switched to semantic versioning, we need to carry epoch from
ceilometer in our provides/obsoletes. No need to add epoch to the package
though

=> Problem is that your main package and  doesn't require
openstack-aodh-{notifier,evaluator} and hence the upgrade path is still
incorrect.
So either we create a subpackage openstack-aodh-alarm-compat requiring all the
packages providing features from openstack-ceilometer-alarm, so we just 


Relevant guidelines
[1]
https://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#.28n:m.29_Many_to_many_replacement

If you're aware of other similar features mapping in ceilometer/aodh, let me
know so we could figure out the upgrade path.

* use versioned python macros

* I exempt this package from newer python guidelines as we need to make sure
that the upgrade path from ceilometer alarm is ok before renaming stuff.
Moreover, openstack-xxx packages will be retired from F24 at a later point.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1243533
[Bug 1243533] (RDO) Tracker: Review requests for new RDO Liberty packages
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Pradeep Kilambi  changed:

   What|Removed |Added

  Flags|needinfo?(pkilambi@redhat.c |
   |om) |



--- Comment #4 from Pradeep Kilambi  ---
(In reply to Haïkel Guémar from comment #3)

Thanks for the review. Replying inline:

> ## Blockers
> 
> 
> * I'll need your help to properly understand ceilometer/aodh relationship
> and figure out the proper upgrade path.
> 
> I pretty much get that openstack-aodh-{notifier,evaluator} services replace
> services with the same in ceilometer.
> Problem is that openstack-ceilometer-{notifier,evaluator} were shipped in a
> single openstack-ceilometer-alarm package, previously.


That is correct. We discussed weather to keep these together or separate. The
reason i split is because these services can be run on thier own nodes. Which
would be hard to do if its a single package? If the recommendation is to keep
these together as part of deprecation then i understand and can change it. 


> 
> => You already obsoletes openstack-ceilometer-alarm but it's unversioned,
> should be:
> Provides: openstack-ceilometer-alarm = 2:%{version}-%{release}
> Obsoletes: openstack-ceilometer-alarm < last n-v-r with epoch of
> openstack-ceilometer-alarm
> 
> As ceilometer switched to semantic versioning, we need to carry epoch from
> ceilometer in our provides/obsoletes. No need to add epoch to the package
> though


Yep understand, reason i dint add it yet is i was not sure which version to put
in. Since liberty is not out yet? Ideally this version should be the final
version of ceilometer-alarm package? Is it ok to add the current version and
update later?


> 
> => Problem is that your main package and  doesn't require
> openstack-aodh-{notifier,evaluator} and hence the upgrade path is still
> incorrect.
> So either we create a subpackage openstack-aodh-alarm-compat requiring all
> the packages providing features from openstack-ceilometer-alarm, so we just 

I was thinking i can make the aodh-common obsolete ceilometer-alarm. Wouldnt
that work? since aodh-common will be required by all sub-packages?


> 
> 
> Relevant guidelines
> [1]
> https://fedoraproject.org/wiki/
> Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#.28n:m.
> 29_Many_to_many_replacement
> 
> If you're aware of other similar features mapping in ceilometer/aodh, let me
> know so we could figure out the upgrade path.
> 
> * use versioned python macros
> 
> * I exempt this package from newer python guidelines as we need to make sure
> that the upgrade path from ceilometer alarm is ok before renaming stuff.
> Moreover, openstack-xxx packages will be retired from F24 at a later point.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550



--- Comment #2 from Pradeep Kilambi  ---
Spec URL: https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh.spec
SRPM URL:
https://pkilambi.fedorapeople.org/openstack-aodh/openstack-aodh-1.0.0-1.fc22.src.rpm

Since we released and tagged 1.0, here is the new updated spec.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Pradeep Kilambi  changed:

   What|Removed |Added

  Flags||fedora-review?



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Pradeep Kilambi  changed:

   What|Removed |Added

   Assignee|nob...@fedoraproject.org|karlthe...@gmail.com



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1243550] Review Request: openstack-aodh - OpenStack Telemetry Alarming

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1243550

Brian Demers brdem...@cisco.com changed:

   What|Removed |Added

 CC||brdem...@cisco.com



--- Comment #1 from Brian Demers brdem...@cisco.com ---
Informal Review:

Guessing this should be added to RDO:
https://openstack.redhat.com/packaging/rdo-packaging.html#_how_to_add_a_new_package_to_rdo_master_packaging

Take a look at:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag for
versioning and dist tag

See https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL
to set Source0 to a URL (or a comment)

Requires and BuildRequires are fragmented, which makes the spec harder to read.

Not sure where the 'ceilometer' group is created, but you might want to make a
note about, I'm assuming it gets created by another openstack rpm, but i'm not
sure.
https://fedoraproject.org/wiki/Packaging:UsersAndGroups

%license is missing

Download and run 'fedora-review -b 1243550' you will see other errors and
messages that  will need to be cleaned up. 
https://fedoraproject.org/wiki/Package_Review_Process

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review