Skip to content

Feature/rate limits#80

Merged
chatterchats merged 5 commits intomasterfrom
feature/rate-limits
Apr 23, 2024
Merged

Feature/rate limits#80
chatterchats merged 5 commits intomasterfrom
feature/rate-limits

Conversation

@dealloc
Copy link
Member

@dealloc dealloc commented Apr 20, 2024

This pull request adds 2 new features:

  • allow reconfiguring the rate limits through appsettings or environment variables, this allows self hosting users to set a custom rate limit
  • allow passing a JWT token that overrides the configured rate limits

You can generate JWT tokens in development by hitting the /dev/token endpoint (endpoint is not available outside DEBUG environments).

Documentation has also been rudimentary updated to explain self hosting users to configure the rate limits.

Additionally, it also updates the documentation to refer users to https://helldivers-2.elixus.be so that if we ever rename the project or move away from Fly.io the URL wouldn't break all applications

@dealloc dealloc added documentation Improvements or additions to documentation feature request This issue requests a feature that doesn't exist (yet) labels Apr 20, 2024
@dealloc dealloc added this to the V1 milestone Apr 20, 2024
@dealloc dealloc requested a review from a team April 20, 2024 11:04
@dealloc dealloc self-assigned this Apr 20, 2024
@dealloc dealloc requested a review from chatterchats as a code owner April 20, 2024 11:04
@dealloc dealloc linked an issue Apr 20, 2024 that may be closed by this pull request
@dealloc
Copy link
Member Author

dealloc commented Apr 20, 2024

One discussion I'm open to is whether the domain should be under elixus.be or if we want to consider a custom domain for hosting this API (and perhaps the other projects in the organisation?)

@dealloc
Copy link
Member Author

dealloc commented Apr 20, 2024

I have also already configured a JWT signing key on Fly
image

lambstream
lambstream previously approved these changes Apr 22, 2024
Copy link
Contributor

@lambstream lambstream left a comment

Choose a reason for hiding this comment

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

Looking spicy, in future we should use a different domain name imho

@dealloc
Copy link
Member Author

dealloc commented Apr 22, 2024

then this shouldn't be merged until domain is agreed upon

@dealloc dealloc force-pushed the feature/rate-limits branch from 6305e3d to a000866 Compare April 22, 2024 20:04
chatterchats
chatterchats previously approved these changes Apr 22, 2024
Copy link
Contributor

@chatterchats chatterchats left a comment

Choose a reason for hiding this comment

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

Good for me, free to not merge until figure out domain stuff

@dealloc
Copy link
Member Author

dealloc commented Apr 23, 2024

@lambstream @chatterchats what domain are we thinking of setting up then?
ideally we'd communicate the new URL as fast as possible to get users migrated ASAP

@lambstream
Copy link
Contributor

@dealloc @chatterchats I think something like helldivers2.app or helldivers2-api.dev

@chatterchats
Copy link
Contributor

chatterchats commented Apr 23, 2024

@dealloc @chatterchats I think something like helldivers2.app or helldivers2-api.dev

image
image
Both are good, and available

I support dev more than app, just because I feel .app should be more targeted towards an application, where as an API feels more like a .dev

@dealloc
Copy link
Member Author

dealloc commented Apr 23, 2024

helldivers2-api.dev or helldivers-2-api.dev (more in line with the org name)

@chatterchats
Copy link
Contributor

I'd say helldivers-2-api.dev if it's between those 2, dash lines up with the github 😂

@dealloc dealloc dismissed stale reviews from chatterchats and lambstream via 7528282 April 23, 2024 18:10
@dealloc dealloc force-pushed the feature/rate-limits branch from a000866 to 7528282 Compare April 23, 2024 18:10
@dealloc
Copy link
Member Author

dealloc commented Apr 23, 2024

helldivers-2-api.dev is good for me, thoughts @lambstream?

PS: fixed merge conflict

@dealloc
Copy link
Member Author

dealloc commented Apr 23, 2024

@chatterchats @lambstream PR should be good to review now

Copy link
Contributor

@chatterchats chatterchats left a comment

Choose a reason for hiding this comment

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

Looks good to me

@chatterchats chatterchats merged commit f754f2b into master Apr 23, 2024
@chatterchats chatterchats deleted the feature/rate-limits branch April 23, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation feature request This issue requests a feature that doesn't exist (yet)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE]: option to increase rate limits

3 participants

Comments