katello icon indicating copy to clipboard operation
katello copied to clipboard

Published 20 hours ago • verified publisher icon Katello

Fixes #35503 - Notify user only when new errata are added to repo

Open hao-yu opened this issue 1 year ago • 1 comments

hao-yu avatar Sep 22 '22 08:09 hao-yu

Issues: #35503

theforeman-bot avatar Sep 22 '22 08:09 theforeman-bot

@pmoravec @wbclark you guys think this idea is workable? If yes then I will un-draft it.

hao-yu avatar Oct 12 '22 11:10 hao-yu

I like that approach, it should definitely fix the repeated issues.

The only concern is how scalable it is - can the dynflow steps handle inputs/outputs with (possibly tens of) thousands of errata IDs?

Is there some limitation, @adamruzicka ?

pmoravec avatar Oct 12 '22 15:10 pmoravec

I have no data to back this claim up, but it should be fine. Of course the more data you push through the slower it will be, but there is no explicit limitation. Unless we get to the ballpark of around tens of megabytes in size, it should be just fine.

adamruzicka avatar Oct 12 '22 17:10 adamruzicka

I think it should be ok. The table ids should be much smaller than the packages data that we used to put in the package profile upload tasks and the content view publish task.

hao-yu avatar Oct 13 '22 00:10 hao-yu

@hao-yu the approach looks nice. do you have bandwidth to finish it at the moment?

wbclark avatar Oct 17 '22 13:10 wbclark

Sounds great to me!

pmoravec avatar Oct 18 '22 06:10 pmoravec

1 test failure seems unrelated to this patch

hao-yu avatar Oct 18 '22 07:10 hao-yu

Tests are green now.

chris1984 avatar Oct 18 '22 18:10 chris1984

I'm running some live tests on this in my devel environment. Will do another review when that is complete.

(If anyone else is interested, I had to rebase this on latest master branch to get it to start)

wbclark avatar Oct 19 '22 18:10 wbclark

My sincere apologies for the delay in finishing this review.

After thoroughly testing this in my development environment, I came up with a few recommendations to slightly improve the UX and mostly to make the behavior around updating the inputs more consistent (in the prior implementation, the run step never executes when contents_changed => false so the post-hoc input truncation never occurs).

To finish before the upcoming stabilization period, I've opened a new PR https://github.com/Katello/katello/pull/10328 which amends the original commit (keeping a single commit and so that @hao-yu 's authorship is preserved) and also adds myself and @pmoravec as co-authors.

A complete write up of the changes is included, along with my testing steps.

I'm requesting to please close this PR and continue the discussion at #10328 ... thanks to all for the very helpful discussion.

wbclark avatar Oct 27 '22 03:10 wbclark

Closing this since https://github.com/Katello/katello/pull/10328 has merged.

ianballou avatar Nov 01 '22 16:11 ianballou