-
Notifications
You must be signed in to change notification settings - Fork 46
Patch 1 #3
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
Patch 1 #3
Conversation
coc/client.py
Outdated
| This indicates whether the client should by default update your API Token | ||
| when your IP Adress changes. | ||
| key_count: Optional[:class:`int`] | ||
| The ammount of keys to use for this client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo "ammount" --> "amount"
coc/client.py
Outdated
| More can be found on dynamic IP Adresses this API uses at: | ||
| https://developer.clashofclans.com | ||
| key_names: Optional[:class:`str`] | ||
| The ammount of keys to use for this client. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Default name for keys created to use for this client" or smthing
coc/client.py
Outdated
| if update_tokens and not has_auth: | ||
| raise RuntimeError('An email and password must be set if update_tokens is True') | ||
| if not key_count == correct_key_count: | ||
| raise RuntimeError("Token count must be within {}-{}".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consistent with use of "key" rather than token
coc/client.py
Outdated
| async def on_token_reset(self, new_token): | ||
| """Event: called when the client's token is reset. | ||
| async def on_key_reset(self, new_key): | ||
| """Event: called when the client's key is reset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're dealing with multiple keys should this line look something like "called when one of the clients keys..." or "called when a client key..."
coc/client.py
Outdated
| password=password, update_token=True) | ||
| def __init__(self, key, email, password): | ||
| super().__init__(key=key, email=email, | ||
| password=password, update_key=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update_key/token is no longer a kwarg
| self.key_count = key_count | ||
|
|
||
| asyncio.ensure_future(self.login(tokens)) | ||
| loop.run_until_complete(self.get_keys()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the whole loop.run_until_complete() was what was bugging out tuba's error with reloading client in same script.....while I dont know what the downside of using ensure_future is (apart from not being able to return anything), afaik it was working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think I change that line because of not knowing you used self.request to access the api/key portal too I think I can safely change that back my bad
coc/http.py
Outdated
| self._keys.append(await self.create_key( | ||
| cookies,self.key_names, key_description, [ip])) | ||
| else: | ||
| await self.__session.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be better to call close() in case we want to pass anything else there in future, and it also checks if session is actually active (errors if not?)
coc/http.py
Outdated
| if self.update_tokens: | ||
| log.info('Resetting Clash of Clans token') | ||
| await self.reset_token(token) | ||
| if self.update_keys: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no longer a kwarg in client/http. do we need to bring it back?
coc/http.py
Outdated
| @staticmethod | ||
| def create_cookies(response_dict, session): | ||
| return "session={};game-api-url={};game-api-token={}".format( | ||
| return "session={};game-api-url={};game-api-key={}".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you start doing regex change token to key? lol? this is how web stitches together cookies, so unless something changed, or there is a better way of doing this, pretty sure it was correct before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a ctrlf to change token to key and that was after my tests oops I'll change that back thanks
| if not has_auth: | ||
| raise RuntimeError('An email and password must be set if no tokens are provided') | ||
| self.loop = loop or asyncio.get_event_loop() | ||
| correct_key_count = max(min(KEY_MAXIMUM, key_count), KEY_MINIMUM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this line work? i havent tested but in my head it doesnt.....or at least what line 111 does below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm i missed the not bit
| return "session={};game-api-url={};game-api-token={}".format( | ||
| session, | ||
| response_dict['swaggerUrl'], | ||
| response_dict['temporaryAPIToken'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same problem as above - called temporaryAPIToken in response
| log.info('Resetting Clash of Clans key') | ||
| await self.reset_key(key) | ||
| return await self.request(route, **kwargs) | ||
| log.info('detected invalid key, however client requested not to reset.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant?
coc/http.py
Outdated
| from itertools import cycle | ||
| from datetime import datetime | ||
| from .errors import HTTPException, Maitenance, NotFound, InvalidArgument, InvalidToken, Forbidden | ||
| from .errors import HTTPException, Maitenance, NotFound, InvalidArgument, InvalidKey, Forbidden |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to change this in .errors, .init, and docs (docs/api.rst) (renaming InvalidToken --> InvalidKey)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for it I just got rid of it
So I tested this and it's fully working. Also I changed from using the word token to using key as it matches what they call it in the developer portal.