Skip to content

Comment reactions#53

Open
colelloa wants to merge 11 commits into
gousiosg:masterfrom
colelloa:comment-reactions
Open

Comment reactions#53
colelloa wants to merge 11 commits into
gousiosg:masterfrom
colelloa:comment-reactions

Conversation

@colelloa

Copy link
Copy Markdown
Contributor

Add reactions to issue comments

-change data model, add reaction fields to issue_comments table
-alter paged_api_request in api_client.rb so that it can handle different media types
-alter ensure_issue_comment in ghtorrent.rb so that it inserts new fields or updates fields if necessary
-alter retrieve_issue_comment in retriever.rb so that it uses the media type necessary to get the reaction fields

  • add Sequel.split_symbols = true to db connection to quell deprecation warnings ***this line may not be necessary given most recent commits to master

Comment thread lib/ghtorrent/ghtorrent.rb Outdated
:comment_id => comment_id)
else
debug "Issue comment #{issue_comment_str} exists"
if ! (curcomment[:like] == reactions['+1'] or

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm having a lot of trouble testing this code block because of the constraint violations mentioned in #52

@colelloa

colelloa commented Nov 3, 2017

Copy link
Copy Markdown
Contributor Author

more changes:
add logging to help debug events
fix following bug by altering event_processing.rb (side effects?):

WARN, 2017-11-03T15:30:21-04:00, ghtorrent -- full_repo_retriever.rb: Error processing event. Type: PushEvent, ID: 6752855107.
Error: PG::UndefinedColumn: ERROR: column project_commits.project_id) does not exist
LINE 1: ...s", "projects", "users" WHERE (("projects"."id" = "project_c...
^
HINT: Perhaps you meant to reference the column "project_commits.project_id".

/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in async_exec' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in block in execute_query'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/database/logging.rb:45:in log_connection_yield' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:166:in execute_query'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:153:in block in execute' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:129:in check_disconnect_errors'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:153:in execute' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:515:in _execute'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:327:in block (2 levels) in execute' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:537:in check_database_errors'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:327:in block in execute' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/database/connecting.rb:301:in block in synchronize'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/connection_pool/single.rb:31:in hold' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/database/connecting.rb:301:in synchronize'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:327:in execute' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/dataset/actions.rb:1135:in execute'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/adapters/postgres.rb:680:in fetch_rows' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/dataset/actions.rb:155:in each'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/dataset/actions.rb:52:in block in all' /Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/dataset/actions.rb:1052:in _all'
/Users/acolello/.rvm/gems/ruby-2.1.10/gems/sequel-4.49.0/lib/sequel/dataset/actions.rb:52:in all' /Users/acolello/git/ghtorrent/lib/ghtorrent/event_processing.rb:52:in block in PushEvent'
/Users/acolello/git/ghtorrent/lib/ghtorrent/event_processing.rb:42:in each' /Users/acolello/git/ghtorrent/lib/ghtorrent/event_processing.rb:42:in PushEvent'
/Users/acolello/git/ghtorrent/lib/ghtorrent/commands/full_repo_retriever.rb:114:in block in retrieve_full_repo' /Users/acolello/git/ghtorrent/lib/ghtorrent/commands/full_repo_retriever.rb:108:in each'
/Users/acolello/git/ghtorrent/lib/ghtorrent/commands/full_repo_retriever.rb:108:in retrieve_full_repo' /Users/acolello/git/ghtorrent/lib/ghtorrent/commands/ght_retrieve_repo.rb:31:in go'
/Users/acolello/git/ghtorrent/lib/ghtorrent/command.rb:66:in run' bin/ght-retrieve-repo:6:in

'.

@colelloa

colelloa commented Nov 3, 2017

Copy link
Copy Markdown
Contributor Author

still awaiting resolution of #52 before able to fully test

@gousiosg

gousiosg commented Nov 6, 2017

Copy link
Copy Markdown
Owner

Hi @colelloa, thanks for fixing the bug in full_repo_retriever.rb. I 've backported your fixes to master and put them in production; it was necessary as we where loosing events. You will get a conflict if you sync to master.

@gousiosg

gousiosg commented Nov 6, 2017

Copy link
Copy Markdown
Owner

The Sequel.split_symbols = true line should be redundant now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants