Fixes #36622 - Mark needs_publish after reindexing #10668
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Issues: #36622 |
ianballou
reviewed
Aug 2, 2023
app/models/katello/content_view.rb
Outdated
| # d) If repo was indexed after cv publish. This can happen under 3 cases: | ||
| # i) Index runs because last index(before publish) had failed and repo is picked up for index even if pulp publication hasn't changed. | ||
| # ii) Complete sync runs or sync adds/removes new content (Already true because new pulp publication/version gets created) | ||
| # iii) repo.index_content(force_index: true) is run. (This doesn't necessarily indicate contents changed. Corner case where we play safe and return true) |
There was a problem hiding this comment.
Suggested change
| # iii) repo.index_content(force_index: true) is run. (This doesn't necessarily indicate contents changed. Corner case where we play safe and return true) | |
| # iii) repo.index_content is run. (This doesn't necessarily indicate contents changed. Corner case where we play safe and return true) |
From the code and testing it looks like force_index: true isn't needed to make the CVV say that it needs an update?
There was a problem hiding this comment.
Hah..I had that meant for the action Repository::IndexContent which takes that param but then I moved the indexed_at datetime update to index_content method to be more accurate..Will update the text here.
ianballou
reviewed
Aug 2, 2023
sjha4
force-pushed
the
track_indexing_changes
branch
from
f3c3daf
to
a975127
Compare
wbclark
pushed a commit
to wbclark/katello
that referenced
this pull request
Sep 7, 2023
(cherry picked from commit 4e3d804)
wbclark
pushed a commit
that referenced
this pull request
Sep 27, 2023
(cherry picked from commit 4e3d804)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the changes introduced in this pull request?
Set needs_publish true when repo is indexed.
Considerations taken when implementing this change?
An indexing in katello will now show up as needs_publish=true in the CV. Thanks to optimizations, when repo contents haven't changed, indexing is skipped so in most cases, CV won't set needs_publish=true when contents haven't changed. But if a forced index is run on repo wthout adding new content, CV will have needs_publish true. You can see this by going to console and running: repo.index_content manually.
What are the testing steps for this pull request?
The reproducer steps are a little weird because you need to fake an indexing error in katello to reproduce this.
1: (Fake the error)
In app/models/katello/repository.rb , method index_content add
fail _('Forced error')as first line. (Downstream requires server restart to reload updates:foreman-maintain service restart)2. Restart the server and create a new yum repo. Do not sync the repo yet.
3. Add new repo to a CV and publish the Content View.
4. Sync the repo. Skip the indexing failure from Dynflow console of the task and let the task finish with warning. You shouldn't see any content in the repo.
5. Publish the CV again.
6. Remove the
fail _('Forced error')from app/models/katello/repository.rb , method index_content7. Restart server (Downstream requires server restart to reload updates:
foreman-maintain service restart)8. Sync the repo now.
9. You should see the CV have the needs_publish icon show up once contents are added correctly in katello.