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

Discord Rate Limiting (429 Error code) handling #901

Merged
merged 2 commits into from Jul 9, 2023
Merged

Conversation

caronc
Copy link
Owner

@caronc caronc commented Jul 9, 2023

Description:

Related issue (if applicable): n/a

On Discord it was brought forth by a user in the the #support channel that the HTTP Error 429 (Rate Limit Exceeded) can be handled by Apprise. The 429 message also returns when it's okay to try submitting your message a second time.

This enhancement allows 1 additional try to be made to submit a Discord message relative to the time the previous error code said it would be safe to do so.

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • No lint errors (use flake8)
  • 100% test coverage

Testing

Anyone can help test this source code as follows:

# Create a virtual environment to work in as follows:
python3 -m venv apprise

# Change into our new directory
cd apprise

# Activate our virtual environment
source bin/activate

# Install the branch
pip install git+https://github.com/caronc/apprise.git@discord-rate-limits

# Test out the changes with the following command:
apprise -t "Test Title" -b "Test Message" \
  "discord://credentails"

# Or better yet; spam discord to test this out:
while [ 1 -eq 1 ]; do
   apprise -v -t "Test Title" -b "Test Message" \
     "discord://credentails"
done

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (78f5693) 99.98% compared to head (515beb2) 99.98%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #901   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         120      120           
  Lines       15997    16017   +20     
  Branches     3268     3272    +4     
=======================================
+ Hits        15995    16015   +20     
  Misses          1        1           
  Partials        1        1           
Impacted Files Coverage Δ
apprise/plugins/NotifyDiscord.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jeankhawand jeankhawand mentioned this pull request Jul 9, 2023
2 tasks
@jeankhawand
Copy link

I tested it. seems working! how can I test it using apprise-api? cause currently I used apprise cli which I guess apprise-api implement under the hood right?
Thanks for your quick response!

@caronc
Copy link
Owner Author

caronc commented Jul 9, 2023

You'll need to pull this branch into your Docker apprise-api container.

I think you may need to tailor the Dockerfile a bit so it uses this branch.

Something like:

ARG ARCH
FROM ${ARCH}python:3.11-slim

# set version label
ARG BUILD_DATE
ARG VERSION
LABEL build_version="Apprise API version:- ${VERSION} Build-date:- ${BUILD_DATE}"
LABEL maintainer="Chris-Caron"

# set environment variables
ENV PYTHONDONTWRITEBYTECODE 1
ENV PYTHONUNBUFFERED 1
ENV APPRISE_CONFIG_DIR /config
ENV APPRISE_ATTACH_DIR /attach
ENV APPRISE_PLUGIN_PATHS /plugin

# Install nginx, supervisord, and cryptography dependencies
RUN apt-get update -qq && \
    apt-get install -y -qq nginx supervisor \
    build-essential libffi-dev libssl-dev cargo pkg-config python3-dev rustc

# Cryptography documents that the latest version of pip3 must always be used
RUN pip3 install --upgrade pip

# Install requirements and gunicorn
COPY ./requirements.txt /etc/requirements.txt
RUN pip3 install --no-cache-dir -q -r /etc/requirements.txt gunicorn --no-binary cryptography

## NEW INSERT - 2023.07.09 for Discord Branch Testing ##
RUN apt-get install -y git && \
   pip3 install --upgrade --force-reinstall \
   git+https://github.com/caronc/apprise/@discord-rate-limits

# Copy our static content in place
COPY apprise_api/static /usr/share/nginx/html/s/

# set work directory
WORKDIR /opt/apprise

# Copy over Apprise API
COPY apprise_api/ webapp

# Cleanup
RUN apt-get remove -y -qq build-essential libffi-dev libssl-dev python3-dev cargo rustc pkg-config && \
    apt-get clean autoclean && \
    apt-get autoremove --yes && \
    rm -rf /var/lib/{apt,dpkg,cache,log}/

# Configuration Permissions (to run nginx as a non-root user)
RUN umask 0002 && \
    mkdir -p /attach /config /plugin /run/apprise && \
    chown www-data:www-data -R /run/apprise /var/lib/nginx /attach /config /plugin

# Handle running as a non-root user (www-data is id/gid 33)
USER www-data
VOLUME /config
VOLUME /attach
VOLUME /plugin
EXPOSE 8000
CMD ["/usr/bin/supervisord", "-c", "/opt/apprise/webapp/etc/supervisord.conf"]

The Above might work. You'll need to build that instead (docker build)

@caronc caronc merged commit a94700b into master Jul 9, 2023
14 checks passed
@caronc caronc deleted the discord-rate-limits branch August 14, 2023 01:06
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

3 participants