refactor multithreading into WattTimeBase#35
Merged
sam-watttime merged 2 commits intofuture-releasefrom Mar 12, 2025
Merged
Conversation
jcofield
reviewed
Feb 12, 2025
| self._rate_limit_lock = ( | ||
| threading.Lock() | ||
| ) # prevent multiple threads from modifying _last_request_times simultaneously | ||
| self._rate_limit_condition = threading.Condition(self._rate_limit_lock) |
Contributor
There was a problem hiding this comment.
Is there anyway to move this into the method(s) that need it to avoid having a loosely defined class structure? If not me may need to consider some more structure, like methods to set and access these attributes.
jcofield
reviewed
Feb 12, 2025
| else: | ||
| rsp.raise_for_status() | ||
| return rsp.json() | ||
| j = self._make_rate_limited_request(url, params=params) |
Contributor
There was a problem hiding this comment.
Is this a change in functionality? If so can can avoid doing it during the refactor?
This was referenced Mar 14, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a refactor of the multithreading changes introduced in #34
Notably, we wanted to remove the use of a
MixInobject. This was achieved by adding new methods toWattTimeBaseincluding:_make_rate_limited_request: this should be used in place ofrequest.get()across all child classes. For both single threaded and multithreaded applications, it will ensure that no more than 10 requests per second can be submitted, and if they are, it will sleep (0.1s) until the limit is no longer violated. Significant DRY code was moved into this method toget,raise_for_status(), raise any exceptions, and print any warnings._apply_rate_limitis used bymake_rate_limited_request. This method checks a class attribute tracking the times of the last submitted requests, and ensures that no more thanself.rate_limitare submitted per second. This is achieved by sleeping in a singlethreaded context, and using threading.Condition when multithreading to wait the appropriate amount of time. No use of thethreadingmodule is required at all ifself.multithreaded==False_fetch_datawill call_make_rate_limited_requestin a for loop with a list of parameters (e.g. chunks of dates). if multithreading is enabled, then a threadpool executor is used and futures are examinedas_completed()Sorry for the messy git history here... I was working with this repo as
upstreamand when I went to push to a new branch (multithreading-refactor) it pushed tomain. I wouldn't think this would be possible due to branch protection rules... I'll look into how to tighten those up. In any case, I reverted the commit onmain, rebasedmaininto this branch and then cherry-picked the commit to show the differences here while maintaining the history.