-
-
Notifications
You must be signed in to change notification settings - Fork 927
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.
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
Update linkset homepage from gemspec only after version gets indexed #5249
base: master
Are you sure you want to change the base?
Update linkset homepage from gemspec only after version gets indexed #5249
Conversation
In current implementation, version.reload.indexed? is always false, so gems that only use spec.homepage to specify a homepage will never have a linkset created/updated Signed-off-by: Samuel Giddins <[email protected]>
f22029d
to
1952277
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing blocking, but curious about a few things.
def update_attributes_from_gem_specification!(version, spec) | ||
Rubygem.transaction do | ||
save! | ||
update_versions! version, spec | ||
update_dependencies! version, spec | ||
update_linkset! spec if version.reload.latest? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we never update the linkset now. Is that expected?
def perform(version:) | ||
return unless version.latest? && version.indexed? | ||
|
||
gem = RubygemFs.instance.get("gems/#{version.gem_file_name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only job that redownloads the gem besides the content storage job. Could it just grab this info on push?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right now we don't pass anything besides the version itself to the "after write" set of jobs, so I kept in keeping with that
homepage = package.spec.homepage | ||
|
||
version.rubygem.linkset ||= Linkset.new | ||
version.rubygem.linkset.update!(home: homepage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martinemde this is where we update the linkset
Does this fix #4924 🎉? I think I came to a similar conclusion (linkset is never updated) in #4924 (comment). |
@deivid-rodriguez it should! |
In current implementation, version.reload.indexed? is always false, so gems that only use spec.homepage to specify a homepage will never have a linkset created/updated
Signed-off-by: Samuel Giddins [email protected]