Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove keywords tagger and summary tasks #742

Merged
merged 5 commits into from
Nov 26, 2018
Merged

remove keywords tagger and summary tasks #742

merged 5 commits into from
Nov 26, 2018

Conversation

msehnout
Copy link
Contributor

@msehnout msehnout force-pushed the remove-keywords-workers branch from 44bd7ed to ee19260 Compare November 20, 2018 16:13
@msehnout msehnout force-pushed the remove-keywords-workers branch from ee19260 to 48f901c Compare November 21, 2018 12:34
@msehnout msehnout requested a review from msrb November 21, 2018 12:38
msrb
msrb requested changes Nov 21, 2018
View reviewed changes
Copy link
Member

@msrb msrb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note there is also hack/install_tagger.sh which won't be needed when we remove these tagging tasks from here.

f8a_worker/dispatcher/flows/bayesianPackageTaggingFlow.yml Show resolved Hide resolved
f8a_worker/dispatcher/flows/keywordsSummaryFlow.yml Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #742 into master will increase coverage by 0.89%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
+ Coverage   61.53%   62.42%   +0.89%     
==========================================
  Files          76       71       -5     
  Lines        5311     5097     -214     
==========================================
- Hits         3268     3182      -86     
+ Misses       2043     1915     -128
Impacted Files Coverage Δ
f8a_worker/dispatcher/selective.py 25% <0%> (ø) ⬆️
f8a_worker/storages/s3.py 44.26% <0%> (+1.63%) ⬆️

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 06da737...7f829e6. Read the comment docs.

@centos-ci
Copy link
Collaborator

@msehnout Your image is available in the registry: docker pull quay.io/openshiftio/rhel-bayesian-cucos-worker:SNAPSHOT-PR-742

@msehnout
Copy link
Contributor Author

According to my testing, it doesn't seem to break the workflow.

@msehnout
Copy link
Contributor Author

@msrb review?

msrb
msrb requested changes Nov 23, 2018
View reviewed changes
Copy link
Member

@msrb msrb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you've missed the hack/install_tagger.sh file. We don't need the tool in the image if we are removing all tasks which depend on it.

Otherwise this looks OK-ish 😉

@msehnout
Copy link
Contributor Author

For some reason, I thought you want me to keep it. It should be fine by now.

@centos-ci
Copy link
Collaborator

@msehnout Your image is available in the registry: docker pull quay.io/openshiftio/rhel-bayesian-cucos-worker:SNAPSHOT-PR-742

@msrb
Copy link
Member

msrb commented Nov 23, 2018

👍 Thanks @msehnout 😉 Let me test locally once... I think you can drop the WIP flag. WDYT?

@msehnout msehnout changed the title [WIP] remove keywords tagger and summary tasks remove keywords tagger and summary tasks Nov 23, 2018
@msehnout
Copy link
Contributor Author

Sure, not a bad idea!

@msehnout msehnout requested a review from humaton November 26, 2018 11:26
humaton
humaton reviewed Nov 26, 2018
View reviewed changes
Copy link
Contributor

@humaton humaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@msehnout msehnout merged commit cfffcb8 into fabric8-analytics:master Nov 26, 2018
yzainee-zz pushed a commit to yzainee-zz/fabric8-analytics-worker that referenced this pull request Nov 28, 2018
@msehnout
Merge pull request fabric8-analytics#742 from msehnout/remove-keyword…
77d3208 
…s-workers

remove keywords tagger and summary tasks

initial commit for git operations addition in worker

adding logs

lint

file rectified

rectified queue name

added functionanilty for git operations

lint

rectified clone

additional logic for clone

token rectified

gh username rectified

rectified gemini call

added logs

rectified parser logic

modified logic for gemini

host resolved

Added test cases
yzainee-zz pushed a commit to yzainee-zz/fabric8-analytics-worker that referenced this pull request Nov 29, 2018
@msehnout
# This is a combination of 17 commits.
a4b3e9b 
# This is the 1st commit message:

Merge pull request fabric8-analytics#742 from msehnout/remove-keywords-workers

remove keywords tagger and summary tasks
# This is the commit message fabric8-analytics#2:

initial commit for git operations addition in worker

# This is the commit message fabric8-analytics#3:

adding logs

# This is the commit message fabric8-analytics#4:

lint

# This is the commit message fabric8-analytics#5:

file rectified

# This is the commit message fabric8-analytics#6:

rectified queue name

# This is the commit message fabric8-analytics#7:

added functionanilty for git operations

# This is the commit message fabric8-analytics#8:

lint

# This is the commit message fabric8-analytics#9:

rectified clone

# This is the commit message fabric8-analytics#10:

additional logic for clone

# This is the commit message fabric8-analytics#11:

token rectified

# This is the commit message fabric8-analytics#12:

gh username rectified

# This is the commit message fabric8-analytics#13:

rectified gemini call

# This is the commit message fabric8-analytics#14:

added logs

# This is the commit message fabric8-analytics#15:

rectified parser logic

# This is the commit message fabric8-analytics#16:

modified logic for gemini

# This is the commit message fabric8-analytics#17:

host resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humaton humaton humaton approved these changes

@msrb msrb msrb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@msehnout @codecov-io @centos-ci @msrb @humaton