From 28320b901fc4aeada09f77eb6051679c6493af95 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Thu, 19 May 2022 14:25:42 +0300 Subject: [PATCH 01/12] update error handling --- 0pdd.rb | 2 +- objects/clients/github.rb | 2 -- objects/tickets/github_tickets.rb | 2 +- test/test_0pdd.rb | 4 +++- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/0pdd.rb b/0pdd.rb index 8eeb4828..26c6fea5 100644 --- a/0pdd.rb +++ b/0pdd.rb @@ -228,7 +228,7 @@ xml = repo.xml xml.xpath('//processing-instruction("xml-stylesheet")').remove xml.to_s - rescue StandardError + rescue Exec::Error error 400, "Could not get snapshot for #{name}" end end diff --git a/objects/clients/github.rb b/objects/clients/github.rb index 955e6951..3f145d90 100644 --- a/objects/clients/github.rb +++ b/objects/clients/github.rb @@ -45,7 +45,5 @@ def self.new(config = {}) puts "#{tp.defined_class}##{tp.method_id}()" if tp.defined_class == client.class end.enable client - rescue Octokit::NotFound - puts 'Issue with account not found' end end diff --git a/objects/tickets/github_tickets.rb b/objects/tickets/github_tickets.rb index ceb06dfa..85a3897f 100644 --- a/objects/tickets/github_tickets.rb +++ b/objects/tickets/github_tickets.rb @@ -40,7 +40,7 @@ def notify(issue, message) "@#{@github.issue(@repo, issue)['user']['login']} #{message}" ) rescue Octokit::NotFound => e - puts "The issue most probably is not found, can't coment: #{e.message}" + puts "The issue most probably is not found, can't comment: #{e.message}" end def submit(puzzle) diff --git a/test/test_0pdd.rb b/test/test_0pdd.rb index f8ba7aec..d3e46867 100644 --- a/test/test_0pdd.rb +++ b/test/test_0pdd.rb @@ -101,7 +101,9 @@ def test_renders_html_puzzles end def test_snapshots_unavailable_repo - get('/snapshot?name=yegor256/0pdd_foobar_unavailable') + assert_nothing_raised(Exec::Error) do + get('/snapshot?name=yegor256/0pdd_foobar_unavailable') + end assert(last_response.status == 400) end From 970cbb1cc218d1d88ae9e127a895972d3dd0be03 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Mon, 23 May 2022 11:24:04 +0300 Subject: [PATCH 02/12] [#312] specify error caught --- 0pdd.rb | 4 ++-- test/test_0pdd.rb | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/0pdd.rb b/0pdd.rb index 746050ae..3e78364b 100644 --- a/0pdd.rb +++ b/0pdd.rb @@ -228,8 +228,8 @@ xml = repo.xml xml.xpath('//processing-instruction("xml-stylesheet")').remove xml.to_s - rescue StandardError - error 400, "Could not get snapshot for #{name}" + rescue Exec::Error => e + error 400, "Could not get snapshot for #{name}: #{e.message}" end end diff --git a/test/test_0pdd.rb b/test/test_0pdd.rb index f8ba7aec..d3e46867 100644 --- a/test/test_0pdd.rb +++ b/test/test_0pdd.rb @@ -101,7 +101,9 @@ def test_renders_html_puzzles end def test_snapshots_unavailable_repo - get('/snapshot?name=yegor256/0pdd_foobar_unavailable') + assert_nothing_raised(Exec::Error) do + get('/snapshot?name=yegor256/0pdd_foobar_unavailable') + end assert(last_response.status == 400) end From 76f149578306edcaa8362dcc9891a0adbebdc236 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Mon, 23 May 2022 20:35:05 +0300 Subject: [PATCH 03/12] [#312] update todo description --- 0pdd.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0pdd.rb b/0pdd.rb index 3e78364b..c07187a1 100644 --- a/0pdd.rb +++ b/0pdd.rb @@ -429,7 +429,7 @@ def merged(hash) out end -# @todo #41:30min Make this vcs independent. Move to github vsc object. +# @todo #41:30min Make this vcs independent. Move this logic to github vsc object. def repo(json) uri = json['repository']['ssh_url'] || json['repository']['url'] name = json['repository']['full_name'] From f466503f1945c0e9f09ed56a410772de6e4c26dc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 24 May 2022 05:22:19 +0000 Subject: [PATCH 04/12] Bump nokogiri from 1.13.4 to 1.13.6 Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.13.4 to 1.13.6. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.13.4...v1.13.6) --- updated-dependencies: - dependency-name: nokogiri dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- Gemfile | 2 +- Gemfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index cadfd87c..e7a82e3e 100644 --- a/Gemfile +++ b/Gemfile @@ -27,7 +27,7 @@ gem 'glogin', '0.7.0' gem 'haml', '5.2.1' gem 'mail', '2.7.1' gem 'mocha', '1.11.2', require: false -gem 'nokogiri', '1.13.4' +gem 'nokogiri', '1.13.6' gem 'octokit', '4.20.0' gem 'pdd', '0.20.6' gem 'rack', '2.2.3' diff --git a/Gemfile.lock b/Gemfile.lock index e4571488..da83d153 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -74,10 +74,10 @@ GEM multipart-post (2.1.1) mustermann (1.1.1) ruby2_keywords (~> 0.0.1) - nokogiri (1.13.4) + nokogiri (1.13.6) mini_portile2 (~> 2.8.0) racc (~> 1.4) - nokogiri (1.13.4-x86_64-darwin) + nokogiri (1.13.6-x86_64-darwin) racc (~> 1.4) octokit (4.20.0) faraday (>= 0.9) @@ -172,7 +172,7 @@ DEPENDENCIES haml (= 5.2.1) mail (= 2.7.1) mocha (= 1.11.2) - nokogiri (= 1.13.4) + nokogiri (= 1.13.6) octokit (= 4.20.0) pdd (= 0.20.6) rack (= 2.2.3) From eb1c808bc7ee0fba6e710c0419c19245fff0e773 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Tue, 24 May 2022 11:32:22 +0300 Subject: [PATCH 05/12] [#312] undo renaming variable --- objects/log.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/objects/log.rb b/objects/log.rb index 149eb55d..21cfed0b 100644 --- a/objects/log.rb +++ b/objects/log.rb @@ -31,14 +31,13 @@ class Log def initialize(dynamo, repo) @dynamo = dynamo @repo = repo - @id = Base64.encode64(@repo).gsub(%r{[\s=\/]+}, '') end def put(tag, text) @dynamo.put_item( table_name: '0pdd-events', item: { - 'repo' => @id, + 'repo' => @repo, 'time' => Time.now.to_i, 'tag' => tag, 'text' => "#{text} /#{VERSION}" @@ -53,7 +52,7 @@ def get(tag) select: 'ALL_ATTRIBUTES', limit: 1, expression_attribute_values: { - ':r' => @id, + ':r' => @repo, ':t' => tag }, key_condition_expression: 'repo=:r and tag=:t' @@ -67,7 +66,7 @@ def exists(tag) select: 'ALL_ATTRIBUTES', limit: 1, expression_attribute_values: { - ':r' => @id, + ':r' => @repo, ':t' => tag }, key_condition_expression: 'repo=:r and tag=:t' @@ -78,7 +77,7 @@ def delete(time, tag) @dynamo.delete_item( table_name: '0pdd-events', key: { - 'repo' => @id, + 'repo' => @repo, 'time' => time }, expression_attribute_values: { @@ -98,7 +97,7 @@ def list(since = Time.now.to_i) '#time' => 'time' }, expression_attribute_values: { - ':r' => @id, + ':r' => @repo, ':t' => since }, key_condition_expression: 'repo=:r and #time<:t' From ff84a0889f2c69734983d253b163914de3ac5df4 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Tue, 24 May 2022 12:10:59 +0300 Subject: [PATCH 06/12] [#312] use constructor in github client --- 0pdd.rb | 2 +- objects/clients/github.rb | 12 ++++++++---- test/test_github.rb | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/0pdd.rb b/0pdd.rb index 43a358d5..c07187a1 100644 --- a/0pdd.rb +++ b/0pdd.rb @@ -104,7 +104,7 @@ end end set :server_settings, timeout: 25 - set :github, GithubClient.new(config) + set :github, Github.new(config).client set :dynamo, Dynamo.new(config).aws set :glogin, GLogin::Auth.new( config['github']['client_id'], diff --git a/objects/clients/github.rb b/objects/clients/github.rb index 3f145d90..51e0075c 100644 --- a/objects/clients/github.rb +++ b/objects/clients/github.rb @@ -24,14 +24,18 @@ # Github client # API: http://octokit.github.io/octokit.rb/method_list.html # -class GithubClient - def self.new(config = {}) - client = if config['testing'] +class Github + def initialize(config = {}) + @config = config + end + + def client + client = if @config['testing'] require_relative '../../test/fake_github' FakeGithub.new else args = {} - args[:access_token] = config['github']['token'] if config['github'] + args[:access_token] = @config['github']['token'] if @config['github'] Octokit.connection_options = { request: { timeout: 20, diff --git a/test/test_github.rb b/test/test_github.rb index 14e45afa..81332610 100644 --- a/test/test_github.rb +++ b/test/test_github.rb @@ -28,7 +28,7 @@ # License:: MIT class TestGithub < Test::Unit::TestCase def test_configures_everything_right - github = GithubClient.new + github = Github.new.client assert_equal('0pdd', github.user('0pdd')[:login]) end end From 0013056b85f527fb3adaa0c1722578e3847f79a4 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Wed, 25 May 2022 12:37:26 +0300 Subject: [PATCH 07/12] [#312] remove abstract class and fix lint issues --- 0pdd.rb | 24 ++--- objects/vcs/base.rb | 238 ------------------------------------------ objects/vcs/github.rb | 3 +- 3 files changed, 9 insertions(+), 256 deletions(-) delete mode 100644 objects/vcs/base.rb diff --git a/0pdd.rb b/0pdd.rb index 532c4e00..8abf1ca2 100644 --- a/0pdd.rb +++ b/0pdd.rb @@ -430,22 +430,23 @@ def storage(repo) def process_request(json) repo = repo(json) + github = settings.github JobDetached.new( repo, JobCommitErrors.new( name, - settings.github, + github, json['head_commit']['id'], JobEmailed.new( name, - settings.github, + github, repo, JobRecorded.new( name, - settings.github, + github, JobStarred.new( name, - settings.github, + github, Job.new( repo, storage(name), @@ -455,25 +456,16 @@ def process_request(json) CommitTickets.new( name, repo, - settings.github, + github, json['head_commit']['id'], GithubTaggedTickets.new( name, - settings.github, + github, repo, LoggedTickets.new( Log.new(settings.dynamo, name), name, - MilestoneTickets.new( - name, - repo, - settings.github, - GithubTickets.new( - name, - settings.github, - repo - ) - ) + MilestoneTickets.new(name, repo, github, GithubTickets.new(name, github, repo)) ) ) ) diff --git a/objects/vcs/base.rb b/objects/vcs/base.rb deleted file mode 100644 index 37553be8..00000000 --- a/objects/vcs/base.rb +++ /dev/null @@ -1,238 +0,0 @@ -# Copyright (c) 2016-2021 Yegor Bugayenko -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the 'Software'), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. - -# -# AbstractVCS - provides a uniform interface to work with different VCSes -# -class AbstractVCS - attr_reader :is_valid, :repo, :name - - def initialize(client, json, config = {}) - @name = 'NAME OF VCS' - @json = json - @client = client - @config = config - @is_valid = nil - @repo = nil - end - - def issue(_issue_id) - # Input: - # issue_id -> Number | String - # - # Output: - # { - # state: 'closed' | 'open' - # author: { - # id: String, - # username: String, - # }, - # milestone: { - # number: Number, - # title: String, - # }, - # } - raise NotImplementedError, 'Should accept issue_id and return issue data' - end - - def close_issue(_issue_id) - # Input: - # issue_id -> Number | String - # - # Output: - # nil - raise NotImplementedError, 'Should accept issue_id and return void' - end - - def create_issue(_data) - # Input: - # { - # title: String, - # description: String, - # assignee_id?: Integer, - # milestone_id?: Integer, - # labels?: String[] - # } - # - # Output: - # { - # number: String, - # html_url: String, - # } - raise NotImplementedError, 'Should accept data and return created issue' - end - - def update_issue(_issue_id, _data) - # Input: - # issue_id -> Number | String - # data -> { - # title?: String, - # description?: String, - # assignee_id?: Integer, - # milestone_id?: Integer, - # state?: 'open' | 'closed', - # labels?: String[] - # } - # - # Output: - # nil - raise NotImplementedError, 'You must implement this method' - end - - def labels - # Output: - # { - # id: Number, - # name: String, - # color: String, - # }[] - raise NotImplementedError, 'You must implement this method' - end - - def add_label(_label, _color) - # Input: - # label -> String - # color -> String - # - # Output: - # nil - raise NotImplementedError, 'You must implement this method' - end - - def add_labels_to_an_issue(_issue_id, _labels) - # Input: - # issue_id -> Number | String - # labels -> String[] - # - # Output: - # nil - raise NotImplementedError, 'You must implement this method' - end - - def add_comment(_issue_id, _comment) - # Input: - # issue_id -> Number | String - # comment -> String - # - # Output: - # nil - raise NotImplementedError, 'You must implement this method' - end - - def create_commit_comment(_sha, _comment) - # Input: - # sha -> String - # comment -> String - # - # Output: - # { - # html_url: String, - # } - raise NotImplementedError, 'You must implement this method' - end - - def list_commits - # Output: - # { - # sha: String, - # html_url: String, - # }[] - raise NotImplementedError, 'You must implement this method' - end - - def user(_username) - # Input: - # username -> String - # - # Output: - # { - # name: String, - # email: String, - # } - raise NotImplementedError, 'You must implement this method' - end - - def star - # Output: - # nil - raise NotImplementedError, 'You must implement this method' - end - - def repository(_name = nil) - # Input: - # name -> String = nil - # - # Output: - # { - # private: Boolean, - # } - raise NotImplementedError, 'You must implement this method' - end - - def repository_link - # Output: - # link -> String - raise NotImplementedError, 'You must implement this method' - end - - def collaborators_link - # Output: - # link -> String - raise NotImplementedError, 'You must implement this method' - end - - def file_link(_file) - # Input: - # file -> String - # - # Output: - # link -> String - raise NotImplementedError, 'You must implement this method' - end - - def puzzle_link_for_commit(_sha, _file, _start, _stop) - # Input: - # sha -> String - # file -> String - # start -> Number - # stop -> Number - # - # Output: - # link -> String - raise NotImplementedError, 'You must implement this method' - end - - def issue_link(_issue_id) - # Input: - # issue_id -> String - # - # Output: - # link -> String - raise NotImplementedError, 'You must implement this method' - end - - private - - def git_repo - # Output: - # repo -> GitRepo - raise NotImplementedError, 'You must implement this method' - end -end diff --git a/objects/vcs/github.rb b/objects/vcs/github.rb index 1137340f..4441b3ce 100644 --- a/objects/vcs/github.rb +++ b/objects/vcs/github.rb @@ -19,13 +19,12 @@ # SOFTWARE. require 'octokit' -require_relative 'base' require_relative '../git_repo' # # Github VCS # -class GithubRepo < AbstractVCS +class GithubRepo attr_reader :is_valid, :repo, :name def initialize(client, json, config = {}) From 7ba8c2ee27ca7aa7f98727d08e3bbaa3ebfccf8a Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Wed, 25 May 2022 12:52:16 +0300 Subject: [PATCH 08/12] [#312] remove exception swallowing --- objects/vcs/github.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/objects/vcs/github.rb b/objects/vcs/github.rb index 4441b3ce..55319dc0 100644 --- a/objects/vcs/github.rb +++ b/objects/vcs/github.rb @@ -53,10 +53,11 @@ def issue(issue_id) } end + # @todo #312:30min Currently, if 0pdd fails to close an issue it causes all other downstream execution to be skipped + # therefore leaving the job in a non deterministic state. Catch and track the error here to + # prevent this from happening. Also applies to `add_comment(...)` def close_issue(issue_id) @client.close_issue(@repo.name, issue_id) - rescue Octokit::NotFound => e - puts "The issue most probably is not found, can't close: #{e.message}" end def create_issue(data) @@ -87,8 +88,6 @@ def add_labels_to_an_issue(issue_id, labels) def add_comment(issue_id, comment) @client.add_comment(@repo.name, issue_id, comment) - rescue Octokit::NotFound => e - puts "The issue most probably is not found, can't comment: #{e.message}" end def create_commit_comment(sha, comment) From 14d983db4a1a2c680e3393053398322d49c5220f Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Thu, 26 May 2022 17:22:19 +0300 Subject: [PATCH 09/12] [#312] update --- objects/tickets/milestone_tickets.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objects/tickets/milestone_tickets.rb b/objects/tickets/milestone_tickets.rb index 92de7706..9c9a6294 100644 --- a/objects/tickets/milestone_tickets.rb +++ b/objects/tickets/milestone_tickets.rb @@ -52,7 +52,7 @@ def submit(puzzle) `#{parent[:milestone][:title]}` from issue ##{num}." ) end - rescue StandardError => e + rescue Octokit::Error => e @vcs.add_comment( submitted[:number], "For some reason I wasn't able to set milestone \ From fd9e6c409ff9cd50a620740b2d4a9f402ab58e18 Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Thu, 26 May 2022 17:57:21 +0300 Subject: [PATCH 10/12] [#312] use not found error class --- objects/tickets/tagged_tickets.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/objects/tickets/tagged_tickets.rb b/objects/tickets/tagged_tickets.rb index 9499e10b..2a05badc 100644 --- a/objects/tickets/tagged_tickets.rb +++ b/objects/tickets/tagged_tickets.rb @@ -44,7 +44,7 @@ def submit(puzzle) begin needed.each { |t| @vcs.add_label(t, 'F74219') } @vcs.add_labels_to_an_issue(issue_id, tags) - rescue StandardError => e + rescue Octokit::NotFound => e @vcs.add_comment( issue_id, "I can't create #{@vcs.name} labels `#{needed.join('`, `')}`. \ @@ -53,7 +53,7 @@ def submit(puzzle) [list of collaborators](#{@vcs.collaborators_link}):\ \n\n```#{e.class.name}\n#{e.message}\n#{e.backtrace.join("\n")}\n```" ) - rescue StandardError => e + rescue Octokit::NotFound => e @vcs.add_comment( issue_id, "For some reason I wasn't able to add #{@vcs.name} labels \ From 9dd69518d5941b2229f1189137b19059f12e410d Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Fri, 27 May 2022 03:03:56 +0300 Subject: [PATCH 11/12] [#312] revert to general octokit error class --- objects/tickets/tagged_tickets.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objects/tickets/tagged_tickets.rb b/objects/tickets/tagged_tickets.rb index 2a05badc..405f8e7c 100644 --- a/objects/tickets/tagged_tickets.rb +++ b/objects/tickets/tagged_tickets.rb @@ -44,7 +44,7 @@ def submit(puzzle) begin needed.each { |t| @vcs.add_label(t, 'F74219') } @vcs.add_labels_to_an_issue(issue_id, tags) - rescue Octokit::NotFound => e + rescue Octokit::Error => e @vcs.add_comment( issue_id, "I can't create #{@vcs.name} labels `#{needed.join('`, `')}`. \ From a0d77a96d82f187537bdb57d0e71683614cf630b Mon Sep 17 00:00:00 2001 From: Ayomide Bakare Date: Fri, 27 May 2022 15:12:46 +0300 Subject: [PATCH 12/12] [#312] update indentation of puzzle body --- objects/vcs/github.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/objects/vcs/github.rb b/objects/vcs/github.rb index 55319dc0..15589447 100644 --- a/objects/vcs/github.rb +++ b/objects/vcs/github.rb @@ -54,8 +54,8 @@ def issue(issue_id) end # @todo #312:30min Currently, if 0pdd fails to close an issue it causes all other downstream execution to be skipped - # therefore leaving the job in a non deterministic state. Catch and track the error here to - # prevent this from happening. Also applies to `add_comment(...)` + # therefore leaving the job in a non deterministic state. Catch and track the error here to + # prevent this from happening. Also applies to `add_comment(...)` def close_issue(issue_id) @client.close_issue(@repo.name, issue_id) end