-
Notifications
You must be signed in to change notification settings - Fork 369
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
Add Support for Storing Repo Labels #1683
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
class Label < ActiveRecord::Base | ||
has_many :repo_labels | ||
has_many :repos, through: :repo_lables | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
class RepoLabel < ActiveRecord::Base | ||
belongs_to :repo | ||
belongs_to :label | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# frozen_string_literal: true | ||
|
||
# PORO | ||
# This class takes in a repo and fetch all labels for that repo. It then | ||
# creates labels and associates them with the passed in repo | ||
# | ||
# Example: | ||
# | ||
# repo = Repo.first | ||
# assigner = RepoLabelAssigner.new(repo: repo) | ||
# assigner.create_and_associate_labels! | ||
# | ||
class RepoLabelAssigner | ||
def initialize(repo:) | ||
@repo = repo | ||
url = ['repos', repo.user_name, repo.name, 'labels'].join('/') | ||
@github_bub_response = GitHubBub.get(url) | ||
end | ||
|
||
def create_and_associate_labels! | ||
return unless github_bub_response.success? | ||
|
||
remote_labels.each do |label_hash| | ||
label_name = label_hash['name'].downcase | ||
label = Label.where(name: label_name).first_or_create! | ||
repo.repo_labels.where(label: label).first_or_create | ||
end | ||
end | ||
|
||
private | ||
|
||
attr_reader :github_bub_response, :repo | ||
|
||
def remote_labels | ||
Array(github_bub_response.json_body) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
class CreateLabelsTable < ActiveRecord::Migration[6.1] | ||
def change | ||
create_table :labels do |t| | ||
t.string :name, null: false | ||
|
||
t.timestamps | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
class CreateRepoLabelsTable < ActiveRecord::Migration[6.1] | ||
def change | ||
create_table :repo_labels do |t| | ||
t.references :repo, foreign_key: true, null: false | ||
t.references :label, foreign_key: true, null: false | ||
|
||
t.timestamps | ||
end | ||
|
||
add_index :repo_labels, [:repo_id, :label_id], unique: true | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -111,4 +111,11 @@ namespace :schedule do | |||||||||||||||
next unless Date.today.sunday? | ||||||||||||||||
Rake::Task['sitemap:refresh'].invoke | ||||||||||||||||
end | ||||||||||||||||
|
||||||||||||||||
desc 'fetch and assign labels for repos' | ||||||||||||||||
task fetch_labels_and_assign: :environment do | ||||||||||||||||
Repo.find_each(batch_size: 100) do |repo| | ||||||||||||||||
RepoLabelAssigner.new(repo: repo).create_and_associate_labels! | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered creating a new background job here and enqueueing it for 10 repos at a time but worried about rate-limiting and not knowing the concurrency of background processing in production. I also wasn't sure what the total count of repos in production was. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repo count is on the main page it's ~6,000. The way we do API requests is that we rotate through everyone's tokens on the service for each request. CodeTriage/config/initializers/git_hub_bub.rb Lines 48 to 54 in 31bd0e7
It's a good idea not to make a bunch of extra requests, but we're not that constrained. If this is N API requests where N is the number of repos in the system, that's absolutely no problem. |
||||||||||||||||
end | ||||||||||||||||
end | ||||||||||||||||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'test_helper' | ||
|
||
class RepoLabelsAssignerTest < ActiveSupport::TestCase | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Outside of my comfort zone here (I'm used to RSpec) so if this needs to change please let me know! |
||
test '#create_and_associate_labels! creates labels' do | ||
repo = repos(:rails_rails) | ||
VCR.use_cassette('fetch_labels_for_repo', record: :once) do | ||
assigner = RepoLabelAssigner.new(repo: repo) | ||
assert_difference('Label.count', 30) do | ||
assigner.create_and_associate_labels! | ||
end | ||
end | ||
end | ||
|
||
test '#create_and_associate_labels! it does not create existing labels' do | ||
repo = repos(:rails_rails) | ||
Label.create!(name: :autoloading) | ||
VCR.use_cassette('fetch_labels_for_repo', record: :once) do | ||
assigner = RepoLabelAssigner.new(repo: repo) | ||
assigner.create_and_associate_labels! | ||
assert_equal Label.where(name: :autoloading).count, 1 | ||
end | ||
end | ||
|
||
test '#create_and_associate_labels! it does not care about label case' do | ||
repo = repos(:rails_rails) | ||
VCR.use_cassette('fetch_labels_for_repo', record: :once) do | ||
assigner = RepoLabelAssigner.new(repo: repo) | ||
assigner.create_and_associate_labels! | ||
assert_equal Label.where(name: :ActionMailer).count, 0 | ||
end | ||
end | ||
|
||
test '#create_and_associate_labels! associates labels with the repo' do | ||
repo = repos(:rails_rails) | ||
VCR.use_cassette('fetch_labels_for_repo', record: :once) do | ||
assigner = RepoLabelAssigner.new(repo: repo) | ||
assigner.create_and_associate_labels! | ||
end | ||
|
||
assert_equal Label.count, RepoLabel.where(repo: repo).count | ||
end | ||
end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
The issue stated that we wanted to only store
hacktoberfest
related labels. I'm happy to add anext unless
line here if that's still the case but this is pretty simple as is.