Latest Version Changes #235

yzainee-zz · 2019-02-01T11:36:19Z

No description provided.

codecov-io · 2019-02-07T14:41:24Z

Codecov Report

Merging #235 into master will decrease coverage by 0.49%.
The diff coverage is 96.55%.

@@            Coverage Diff            @@
##           master     #235     +/-   ##
=========================================
- Coverage   92.64%   92.15%   -0.5%     
=========================================
  Files          10       10             
  Lines         884      892      +8     
=========================================
+ Hits          819      822      +3     
- Misses         65       70      +5

Impacted Files	Coverage Δ
src/data_importer.py	`90.33% <100%> (+0.14%)`	⬆️
src/graph_manager.py	`100% <100%> (ø)`	⬆️
src/cve.py	`84.25% <100%> (ø)`	⬆️
src/data_source/s3_data_source.py	`94.44% <100%> (ø)`	⬆️
src/rest_api.py	`89.06% <100%> (-1.05%)`	⬇️
src/utils.py	`100% <100%> (ø)`	⬆️
src/graph_populator.py	`96.72% <85.71%> (-1.36%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3bfb0d...099abbf. Read the comment docs.

miteshvp · 2019-02-08T03:54:21Z

src/graph_populator.py

                      "property('{ecosystem}_pkg_count',1)).iterate();" \
                      "graph.addVertex('ecosystem', '{ecosystem}', " \
                      "'name', '{pkg_name}', 'vertex_label', 'Package');}};" \
+                      "pkg.property('latest_version', {latest_version});" \


@yzainee - you are missing ' single quote around latest_version here. That's why tests are failing

msrb

Please squash some of the commits in this PR ("Initial commit" for example).

There are some scary parts that still deal with libio latest version etc., for example:

fabric8-analytics-data-model/src/graph_populator.py

Lines 403 to 405 in 211dfbb

    
           if libio_latest_version != cur_libio_latest_ver and last_updated_flag != 'true': 
        
               prp_package += "pkg.property('latest_version_last_updated', '{}');"\ 
        
                   .format(cur_date)

Are we sure it won't break any use cases?

I think it would be better to do some refactoring and try to figure out from where to call get_latest_versions_for_ep, so we don't need to call it 3 times.

msrb · 2019-02-08T08:40:57Z

Dockerfile.data-model

 # Note: cron daemon ( crond ) will be invoked from within entry point
 # --------------------------------------------------------------------------------------------------------------
+RUN pip3 install git+https://git@github.com/fabric8-analytics/fabric8-analytics-version-comparator.git
+RUN pip3 install git+https://github.com/fabric8-analytics/fabric8-analytics-utils.git


Wouldn't be better to install specific versions of these dependencies?

Yes i will make those changes in the final commit. Still its WIP as the tests are not passing.

msrb · 2019-02-08T08:41:31Z

Dockerfile.data-model.rhel

 # Note: cron daemon ( crond ) will be invoked from within entry point
 # --------------------------------------------------------------------------------------------------------------
+RUN pip3 install git+https://git@github.com/fabric8-analytics/fabric8-analytics-version-comparator.git
+RUN pip3 install git+https://github.com/fabric8-analytics/fabric8-analytics-utils.git@latest


msrb · 2019-02-08T08:42:16Z

requirements.txt

 urllib3==1.24.1           # via botocore, minio, requests
 uuid==1.30
 werkzeug==0.14.1          # via flask, pytest-flask
+git+https://git@github.com/fabric8-analytics/fabric8-analytics-version-comparator.git


If we install the dependencies here, do we need to install them in the Dockerfiles separately?

msrb · 2019-02-08T08:42:44Z

setup.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+
+# Copyright © 2018 Red Hat Inc.


Happy New Year! :)

msrb · 2019-02-08T13:30:31Z

src/graph_populator.py

                str_gremlin += str_gremlin_version
                if not prp_package:
                    # TODO: refactor into the separate module
+                    latest_version = get_latest_versions_for_ep(ecosystem, pkg_name)


Hmm, why do we call get_latest_versions_for_ep on 3 different places in this PR?

This is my biggest issue with this PR :)

The reason is that different APIs are triggering different functional flows, where gremlin queries are getting generated for the package node. So basically thats 2 of them. The 3rd one, construct graph node function, i have added it so that even in cases where we are creating dummy nodes during CVEs etc, the package node should get updated to the latest version. This is an extra check added to make sure that our package node is always updated.

centos-ci · 2019-02-08T17:35:24Z

@yzainee Your image is available in the registry: docker pull quay.io/openshiftio/rhel-bayesian-data-model-importer:SNAPSHOT-PR-235

centos-ci · 2019-02-13T14:13:23Z

@yzainee Your image is available in the registry: docker pull quay.io/openshiftio/rhel-bayesian-data-model-importer:SNAPSHOT-PR-235

Lock to Radon 3 0 1 initial commit initial commit Changes for latest version cico script changed for python3 added a dummy node for latest version pylint tests rectification fixing tests review comments

tisnik · 2019-02-18T07:52:04Z

[test]

yzainee-zz requested a review from msrb February 1, 2019 11:36

miteshvp suggested changes Feb 8, 2019

View reviewed changes

yzainee-zz force-pushed the notify branch from 9d2d4fd to 211dfbb Compare February 8, 2019 08:10

msrb requested changes Feb 8, 2019

View reviewed changes

tisnik and others added 2 commits February 15, 2019 13:07

Merge pull request fabric8-analytics#236 from tisnik/lock-to-radon-3-0-1

595d75e

Lock to Radon 3 0 1 initial commit initial commit Changes for latest version cico script changed for python3 added a dummy node for latest version pylint tests rectification fixing tests review comments

python2 to python3 migration

94b2ca2

yzainee-zz force-pushed the notify branch from 099abbf to 94b2ca2 Compare February 15, 2019 07:40

yzainee-zz changed the title ~~Notify~~ Latest Version Changes Feb 15, 2019

dummy commit. to trigger build

40f4e2f

fabric8-analytics deleted a comment from samuzzal-choudhury Feb 15, 2019

	if libio_latest_version != cur_libio_latest_ver and last_updated_flag != 'true':
	prp_package += "pkg.property('latest_version_last_updated', '{}');"\
	.format(cur_date)

		@@ -0,0 +1,59 @@
		#!/usr/bin/env python3

		# Copyright © 2018 Red Hat Inc.

Latest Version Changes #235

Are you sure you want to change the base?

Latest Version Changes #235

Uh oh!

Conversation

yzainee-zz commented Feb 1, 2019

Uh oh!

codecov-io commented Feb 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msrb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

centos-ci commented Feb 8, 2019

Uh oh!

centos-ci commented Feb 13, 2019

Uh oh!

tisnik commented Feb 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-io commented Feb 7, 2019 •

edited

Loading