-
Notifications
You must be signed in to change notification settings - Fork 45
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
75% usage reminder #245
75% usage reminder #245
Conversation
if cfg.FLASK_DEBUG: | ||
can_excute = True | ||
else: | ||
user_db = UserUsageTracker(max_daily_token=cfg.DAILY_TOKEN_LIMIT) | ||
user_db = UserUsageTracker( | ||
max_daily_token=cfg.DAILY_TOKEN_LIMIT, verbos_logger=usage_logger) |
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.
global rename verbos_logger => verbose_logger
@@ -86,8 +95,27 @@ def get_sum_of_tokens_since_last_reset(self, user_id): | |||
token_sum = sum(item["token"] for item in data_since_last_reset[1:]) | |||
return token_sum | |||
|
|||
def token_left_calculator(self, combined_id): | |||
split_parts = combined_id.split('_') |
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.
Slack user IDs are allowed to contain underscores. We need to change the way we form combined_id so that it still works if the Slack user ID contains an underscore. This change should be global, across the app.
if cfg.FLASK_DEBUG: | ||
can_excute = True | ||
else: | ||
user_db = UserUsageTracker(max_daily_token=cfg.DAILY_TOKEN_LIMIT) |
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.
Document DAILY_TOKEN_LIMIT in sample.env
@@ -28,16 +30,23 @@ def __init__( | |||
self, | |||
db_name=cfg.DB_NAME, | |||
max_daily_token=20000, |
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.
define a constant somewhere for the 20000 value.
I see this value is also used in config/init.py. It should be defined in one place.
s3.download_file(bucket_name, s3_file_key, local_file_path) | ||
def upload_to_s3(self,local_file_path, bucket_name, s3_file_key): | ||
self.verbos_logger = verbos_logger | ||
self.is_reminded = False |
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.
is_reminded is defined here, but it isn't updated anywhere in the code. What is it for?
getattr(cfg, 'DAILY_TOKEN_LIMIT', self.max_daily_token)) | ||
token_left = maximum_daily_token - total_token_since_last_reset | ||
percentage_used = ((float(maximum_daily_token) - float(token_left))*100) / float(maximum_daily_token) | ||
if percentage_used > 75: |
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.
Right now it looks like this logging is going to happen every time we add data about token usage. We could end up with many log lines.
Instead we should give the user a reminder just once per day.
@@ -86,8 +95,27 @@ def get_sum_of_tokens_since_last_reset(self, user_id): | |||
token_sum = sum(item["token"] for item in data_since_last_reset[1:]) | |||
return token_sum | |||
|
|||
def token_left_calculator(self, combined_id): |
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.
Write unit tests for this function
used the verbose logger to log out the usage status.