Slack addon, three different lead states #79
Conversation
| self.api_url, headers=self.token_header, data=self.request_data | ||
| ) | ||
| token = json.loads(response.text)["access_token"] | ||
| continue |
There was a problem hiding this comment.
Please add a asyncio.sleep(WAIT_TIME_ERROR) here - hold back on error (also define a constant near the top of the file).
python/Auth0/main.py
Outdated
| buffer_users.append(result["email"]) | ||
| if not has_more: | ||
| last_user = result["email"] | ||
| await asyncio.sleep(5) |
There was a problem hiding this comment.
| await asyncio.sleep(5) | |
| await asyncio.sleep(WAIT_TIME_ON_USER) |
| ) | ||
| buffer_users.append(result["email"]) | ||
| if not has_more: | ||
| last_user = result["email"] |
There was a problem hiding this comment.
I think it would be better to keep the update time instead of email - users may change and we could just push the changed users as well, but email will do for now.
python/Auth0/shiftarray.py
Outdated
| self.array = [] | ||
|
|
||
| def append(self, value): | ||
| if len(self.array) == 5: |
There was a problem hiding this comment.
Nice! But
| if len(self.array) == 5: | |
| if len(self.array) == self.ARRAY_LENGTH: |
python/Auth0/shiftarray.py
Outdated
| def __init__(self): | ||
| self.array = [] |
There was a problem hiding this comment.
| def __init__(self): | |
| self.array = [] | |
| def __init__(self, length = 5): | |
| self.array = [] | |
| self.ARRAY_LENGTH = length; |
python/Auth0/main.py
Outdated
| except Exception as error: | ||
| raise Exception(f"Config not loaded: {error}") | ||
| return |
There was a problem hiding this comment.
The return statement is not needed after raising an Exception here (same for chimp and Stripe). Its message "Config not loaded:" suggests that there was a problem with the config, but a different, unrelated exception might be raised by get_auth() method.
|
This line seems to be a leftover. In Auth0, you check if an error occured with the string condition but a better way would be to check its status_code, like: The get_auth() is async so I think these: requests could be made asynchronously. Instead of using prints, I think we should stick to the context.logger. Then you could specify log levels explicitly, so in except blocks you would use Little inconsistency - in Auth0, asyncio.gather() is inside the try-except block, but in Stripe it is not. And of course, as @MichalCz stated, constant variables should be introduced!:D |
…mples into feat/slack-addon Added suggested changes
There was a problem hiding this comment.
One more thing I could propose is not passing context, but rather logger because there is no other context usage in ChimpInsert other than for logging.
And what's up with tako? What is it for?
Also note that the:
verified = await requests_async.get(self.verified_url, headers=headers)
users = await requests_async.get(self.users_url, headers=headers)calls are not truly asynchronous (as in the meaning of the word), but at least they are on the event loop now so its all good👌
|
@S4adam you were right ,"tako" is a debug leftover and I accidentally corrected it but in other file |
No description provided.