Skip to content
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 checkpoint feature to find_inactive_members.rb #268

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

asendia
Copy link

@asendia asendia commented Nov 9, 2019

Script find_inactive_members.rb sometimes breaks while parsing large organization either caused by timeout or connection error.
Checkpoint flag --checkpoint helps by storing Octokit API responses as json files in data/, so that during re-run it will first try to use stored json files instead of calling Github API from the beginning.

@spier
Copy link
Contributor

spier commented Mar 20, 2020

Hi @asendia. I like this idea, and I have run into the problem that you are describing myself.

What help do you need to get this PR over the finish line? Do you need a review, or something else?

@jonico
Copy link
Contributor

jonico commented Mar 20, 2020

I would also support this change and merge it once it has been tested by somebody else like @spier . I resolved the conflicts to upstream to the best of my knowledge already.

@spier
Copy link
Contributor

spier commented Mar 20, 2020

I will try this out locally, and leave comments on this PR.

@@ -278,6 +367,12 @@ def member_activity
Octokit.configure do |kit|
kit.auto_paginate = true
kit.middleware = stack if @debug
kit.connection_options = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why you added this configuration?

Copy link
Author

@asendia asendia Apr 20, 2020

Choose a reason for hiding this comment

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

Hi @spier, thank you for taking your time to review my PR. IIRC, the default values for both request.open_timeout and timeout are 60s. I adjusted the value since it was taking a lot of time for me to retry when it failed.
I used this script in conjunction with bash script (I forget the exact script):

while true; do ruby find_inactive_members.rb; sleep 30; done

@spier
Copy link
Contributor

spier commented Mar 20, 2020

I was wondering if instead of creating a custom caching solution, one could also use something like http://octokit.github.io/octokit.rb/#Caching instead? Don't have time right now to try it unfortunately, but maybe over the weekend.

@asendia
Copy link
Author

asendia commented Apr 20, 2020

I was wondering if instead of creating a custom caching solution, one could also use something like http://octokit.github.io/octokit.rb/#Caching instead? Don't have time right now to try it unfortunately, but maybe over the weekend.

I am not too familiar with ruby as you can see from the code in my PR 🙇

My reasons:

  1. If we use eTag based cache it will still call github API during retry and it will still eat the request bandwidth.
  2. Does that cache persist after the script closes? When I used this script I experienced few numbers of unknown errors that made the script force closed.

CMIIW

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.

3 participants