|Summary:||Review Request: vitrage-dashboard Horizon plugin for vitrage|
|Product:||[Community] RDO||Reporter:||Eyal <eyalb1>|
|Component:||Package Review||Assignee:||Matthias Runge <mrunge>|
|Status:||CLOSED NEXTRELEASE||QA Contact:||hguemar|
|Version:||trunk||CC:||apevec, brault, eyalb1, jpena, mrunge|
|Fixed In Version:||Doc Type:||If docs needed, set a value|
|Doc Text:||Story Points:||---|
|Last Closed:||2017-02-16 17:07:38 UTC||Type:||Bug|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Cloudforms Team:||---||Target Upstream Version:|
|Bug Depends On:|
Description Eyal 2016-11-01 14:03:57 UTC
Spec URL: https://github.com/ALU-CloudBand/vitrage-dashboard/blob/master/openstack-vitrage-ui.spec SRPM URL: https://github.com/ALU-CloudBand/vitrage-dashboard/blob/master/openstack-vitrage-ui-1.0.3-dev8.src.rpm Description: The Vitrage Horizon plugin
Comment 1 Matthias Runge 2016-11-02 07:40:51 UTC
sent a pull request to fix minor issues, currently preventing it building.
Comment 2 Eyal 2016-11-02 07:42:31 UTC
merged it thanks
Comment 3 Matthias Runge 2016-11-03 13:33:24 UTC
another pull request. Based on my specfile: https://raw.githubusercontent.com/mrunge/vitrage-dashboard/master/openstack-vitrage-ui.spec I have a copr repo with all vitrage components: https://copr.fedorainfracloud.org/coprs/mrunge/vitrage/
Comment 4 Matthias Runge 2016-11-14 09:42:01 UTC
vitrage-dashboard bundles third party libraries, which are already used by horizon (d3) and others. Please unbundle them. https://github.com/openstack/vitrage-dashboard/tree/master/vitragedashboard/static/vendor
Comment 5 Eyal 2016-11-15 08:03:03 UTC
Hi Can you push the change to openstack gerrit so the ui guys can approve it thanks
Comment 6 Matthias Runge 2016-11-15 08:23:43 UTC
I'd drop the vendor directory completely. That won't be accepted. It requires generating XStatic packages for those deps not already included in OpenStack packages.
Comment 7 Matthias Runge 2016-11-15 13:23:14 UTC
another issue I found from playing around with the package is: API pieces are currently missing
Comment 8 Eyal 2016-11-15 13:33:08 UTC
yes now I see that from the devstack installation https://github.com/openstack/vitrage-dashboard/blob/master/devstack/plugin.sh#L10 the api is copied I guess we need to do the same in the spec file
Comment 9 Matthias Runge 2016-11-15 15:13:40 UTC
There doesn't seem to be a requirement for copying api over. E.g see ironic-ui: https://github.com/openstack/ironic-ui
Comment 10 Matthias Runge 2016-12-08 08:46:37 UTC
since https://github.com/openstack/vitrage-dashboard/commit/7b4cbda9305402fb97714a261a198cf362fb6de2 was merged, we don't need to copy API over any more. Please tag a new release and include that into the package.
Comment 12 Alan Pevec 2017-01-12 18:46:41 UTC
Answering myself, there isn't a release yet which includes Matthias' fix from comment 10. But we can proceed with RDO Trunk review which builds from master. Here is the licensecheck output, project is APLv2 licensed: vitrage-dashboard/.coveragerc: *No copyright* UNKNOWN vitrage-dashboard/.gitreview: *No copyright* UNKNOWN vitrage-dashboard/.jshintrc: *No copyright* UNKNOWN vitrage-dashboard/.mailmap: *No copyright* UNKNOWN vitrage-dashboard/.testr.conf: *No copyright* UNKNOWN vitrage-dashboard/CONTRIBUTING.rst: *No copyright* UNKNOWN vitrage-dashboard/HACKING.rst: *No copyright* UNKNOWN vitrage-dashboard/LICENSE: *No copyright* Apache (v2.0) vitrage-dashboard/MANIFEST.in: *No copyright* UNKNOWN vitrage-dashboard/README.rst: *No copyright* UNKNOWN vitrage-dashboard/babel.cfg: *No copyright* UNKNOWN vitrage-dashboard/openstack-common.conf: *No copyright* UNKNOWN vitrage-dashboard/requirements.txt: *No copyright* UNKNOWN vitrage-dashboard/setup.cfg: *No copyright* Apache vitrage-dashboard/setup.py: Apache (v2.0) GENERATED FILE vitrage-dashboard/test-requirements.txt: *No copyright* UNKNOWN vitrage-dashboard/tox.ini: *No copyright* UNKNOWN
Comment 13 Javier Peña 2017-02-02 10:39:34 UTC
The following files have been reviewed in Gerrit: - Spec: https://raw.githubusercontent.com/rdo-packages/vitrage-dashboard-distgit/rpm-master/openstack-vitrage-ui.spec - SRPM: http://18.104.22.168:8080/v1/AUTH_b50e80d3969f441a8b7b1fe831003e0a/rdoartifacts/67/4667/11/gate/DLRN-rpmbuild/Z20e036e1b2594625bb9a8d818fda84ae/artifacts/centos/repos/b7/3c/b73c3875060c865628316c85386197986df95441_dev/openstack-vitrage-ui-1.1.1-0.20170201134555.b73c387.el7.centos.src.rpm Please note that the SRPM has been generated by DLRN, so some differences are expected (Version, Release and Source0).
Comment 14 Eyal 2017-02-06 11:47:24 UTC
what's the next step ? the ocata and ocata-uc tags were uncommented in rdo.yaml what do I need to do so I can see get the rpms from rdo ?
Comment 15 Javier Peña 2017-02-06 12:55:25 UTC
Matthias, can you have a look at the spec/SRPM, and set rdo-review+ if appropriate?
Comment 16 Matthias Runge 2017-02-07 07:26:32 UTC
comments 4 and 6 are not addressed. could you please include a comment about licenses of bundled libraries? Some of them are a bit dated already. Maybe it makes sense to remove them and to use the ones used in horizon? (e.g d3?)
Comment 17 Matthias Runge 2017-02-07 07:28:50 UTC
to elaborate more: d3 lib is not licensed under ASL 2.0, as hinted in license field in the spec file.
Comment 18 Eyal 2017-02-07 08:14:36 UTC
I updated the spec file with license ASL and BSD https://review.rdoproject.org/r/#/c/4873/
Comment 19 Alan Pevec 2017-02-09 06:40:27 UTC
Matthias, please check PS2 in https://review.rdoproject.org/r/4873
Comment 20 Matthias Runge 2017-02-09 06:56:36 UTC
left my comment on the review.
Comment 21 Javier Peña 2017-02-16 17:07:38 UTC
We now have builds for vitrage-dashboard in the RDO Trunk repos.