Correctly rotate hash keys - #2422
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
I think the intent is good, but this should wait until #2424 is merged so we have a proper scope to put the encounters: the account.
Scanner > account > request > response first parsed by the account used for the request to update its status > response passed to caller.
Sorry, something went wrong.
| from pgoapi.hash_server import BadHashRequestException, HashingOfflineException | ||
|
|
||
| log = logging.getLogger(__name__) | ||
| _key_scheduler = None |
There was a problem hiding this comment.
We shouldn't introduce globals.
Sorry, something went wrong.
|
|
||
| def configure(key_scheduler): | ||
| global _key_scheduler | ||
| _key_scheduler = key_scheduler |
There was a problem hiding this comment.
This method is only here because of the global, we should use a proper scope.
Sorry, something went wrong.
| self.curr_key = self.key_cycle.next() | ||
| return self.curr_key | ||
|
|
||
| def update_hash_stats(self, key, remaining, maximum, expiration): |
There was a problem hiding this comment.
i dont see the request for the token anymore? did you triple check with several keys and instances that none of the values jumps around?
Sorry, something went wrong.
There was a problem hiding this comment.
I did not triple check anything and does not have several keys to test 😄
I am not really sure about what are your fears, the current desing of api does not return the data with the response so it is not thread safe, we just get the data after the request has been done but if there was a context switch the data we retrieve it is always from the last request, at least we are not mixing the key from the request with the data we are reading as they could be different, we just store the data (token and headers) the api has when this is called.
Proper solution would need to change the API to be thread safe.
Sorry, something went wrong.
There was a problem hiding this comment.
yup and because we store just the last request the token is needed to identify which key made the request, otherwise its failing to allocate the correct key for the value, i have some spare time left tomomorrow and will check if this is going ok or not.
Sorry, something went wrong.
| remaining = HashServer.status.get('remaining', 0) | ||
| maximum = HashServer.status.get('maximum', 0) | ||
| expiration = HashServer.status.get('expiration', None) | ||
| _key_scheduler.update_hash_stats(key, remaining, maximum, expiration) |
There was a problem hiding this comment.
@Alderon86 I am getting the key directly from the status so even if it is not the one that made the request we store the data correctly.
Sorry, something went wrong.
|
Closing this as I will no longer be supporting my PRs, if any one wants to take over please do so. |
Sorry, something went wrong.
Description
Previously we had keyscheduler logic all over the place but we can move all to a single point in apiRequest and it makes a lot more sense.
Motivation and Context
Did this changes for the future api but most of them can be ported back to current api so it will rotate the keys more evenly (instead of changing the key per gmo it does change per request).
The next step should be doing the same for proxies buy I wanted this merged before adding more things.
How Has This Been Tested?
Just some minutes in my test instance, but my main instance is running the branch from where I ported the code.
Screenshots (if appropriate):
Types of changes
Checklist: