Skip to content
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

Updated for stability #180

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Updated for stability #180

wants to merge 18 commits into from

Conversation

kayla-tokash
Copy link

Before: it would stop after a few hundred actions because API limits
Now: it will continue to run after an API limit exception, but wait 10 seconds before continuing.

I also updated the "requirements.txt" to work for my system (I'm using OSX, but it should run on linux distributions and WSL)

ilystsov

This comment was marked as outdated.

Copy link

@ilystsov ilystsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid looping eternally by setting maximum retries number

self._remove_comment(item)
if not self._trial_run:
item.delete()
while 1:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while 1:
MAX_RETRIES_COUNT = 5
for retries_count in range(MAX_RETRIES_COUNT):

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering how long the script takes to run, limiting max retries is not ideal. Add an option to run indefinitely if you're really trying to change it. I understand the distaste for while 1 but the real experience is that it will not run to completion if you have a lot of posts because of the rate limiting.

self._logger.debug("Encountered a problem with the API, probably ratelimiting thanks to bad admins")
self._logger.error(f"Exception: {e}")
self._logger.info(f"Waiting {self._batch_cooldown} seconds")
time.sleep(self._batch_cooldown)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
time.sleep(self._batch_cooldown)
time.sleep(self._batch_cooldown)
else:
self.__logger.error(
"Max retry attempts reached. Unable to complete the operation"
)

Copy link

@ilystsov ilystsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use f-strings instead of .format for consistency

shreddit/shredder.py Outdated Show resolved Hide resolved
Copy link

@ilystsov ilystsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split the long string into multiple lines

shreddit/shredder.py Outdated Show resolved Hide resolved
Copy link

@ilystsov ilystsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catch more specific errors

shreddit/shredder.py Outdated Show resolved Hide resolved
kayla-tokash and others added 3 commits November 29, 2023 13:23
Co-authored-by: ilystsov <127615303+ilystsov@users.noreply.github.com>
Co-authored-by: ilystsov <127615303+ilystsov@users.noreply.github.com>
Co-authored-by: ilystsov <127615303+ilystsov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants