Bug 1390608

Summary: Review Request: vitrage-dashboard Horizon plugin for vitrage
Product: [Community] RDO Reporter: Eyal <eyalb1>
Component: Package ReviewAssignee: Matthias Runge <mrunge>
Status: CLOSED NEXTRELEASE QA Contact: hguemar
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: trunkCC: apevec, brault, eyalb1, jpena, mrunge
Target Milestone: ---Flags: mrunge: rdo-review+
Target Release: trunk   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-02-16 17:07:38 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1410778    

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 11 Alan Pevec 2017-01-10 22:45:10 UTC
Was new release tagged as requested in comment 10 ?

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 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.