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.

Sign up for GitHub

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

Jump to bottom

Fix infinitive 401 errors and dynamically adjust parallel threads #52

Merged
merged 16 commits into from
Nov 23, 2018
Merged

Fix infinitive 401 errors and dynamically adjust parallel threads #52

merged 16 commits into from
Nov 23, 2018

Conversation

minidragon88
Copy link
Contributor

@minidragon88 minidragon88 commented Nov 19, 2018 •
edited

Currently, depends on the user account, we could got HTTP 401 Error when we call JIRA APIs so frequently (Although the credential is correct).

This PRs try to solve the above problem by :

  1. Dynamically calculate the number of parallel threads to reduce the risk of 401
  2. Add a rate limiter to limit the number of requests to send per 2 seconds
  3. Retry search only failed issues

tvhung83
tvhung83 reviewed Nov 19, 2018
View reviewed changes
Copy link

@tvhung83 tvhung83 left a comment

Choose a reason for hiding this comment

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

Left some comments, please take a look.

lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
spec/embulk/input/jira_api/client_spec.rb Show resolved Hide resolved
@tvhung83
Copy link

nit: tuning, not turning.

@tvhung83
Copy link

Btw, it'd be nice to have some descriptions:

  • Context, ie. current issue
  • What is the solution this PR introduces

tvhung83
tvhung83 reviewed Nov 19, 2018
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@minidragon88 minidragon88 changed the title Automatically turning number of concurrent threads to find issues Automatically turning number of concurrent threads and add rate limiter to limit the number of concurrent requests Nov 21, 2018
@minidragon88 minidragon88 changed the title Automatically turning number of concurrent threads and add rate limiter to limit the number of concurrent requests Fix infinitive 401 errors and dynamically adjust parallel threads Nov 21, 2018
@minidragon88
Copy link
Contributor Author

@tvhung83 Could you please take a look at the new commit ?

tvhung83
tvhung83 approved these changes Nov 22, 2018
View reviewed changes
Copy link

@tvhung83 tvhung83 left a comment

Choose a reason for hiding this comment

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

LGTM, left a few non-blocking comments, feel free to merge after you address them.

spec/embulk/input/jira_api/client_spec.rb Outdated Show resolved Hide resolved
spec/embulk/input/jira_api/client_spec.rb Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Outdated Show resolved Hide resolved
lib/embulk/input/jira_api/client.rb Show resolved Hide resolved
@minidragon88 minidragon88 merged commit 66b74de into master Nov 23, 2018
@minidragon88 minidragon88 deleted the reduce_concurrent_threads_in_search_issue branch November 23, 2018 08:05
@minidragon88 minidragon88 mentioned this pull request Feb 13, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvhung83 tvhung83 tvhung83 approved these changes

@kietdo360 kietdo360 Awaiting requested review from kietdo360

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@minidragon88 @tvhung83