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

Expand Auto Settings #486

Closed
wants to merge 182 commits into from
Closed

Conversation

cemathey
Copy link
Collaborator

This is a big one so it'll be hard to review.

I didn't explicitly name all of the arguments for the set_ user config endpoints, I cheated and just passed **kwargs straight through, so it won't auto expose the parameters, but it would have been dramatically more work to do so. If we're happy with all of these changes it wouldn't be hard to go back and explicitly define them later.

  • Create a sub class of Rcon called RconAPI and moves almost all of the endpoints that were separately defined into it so that they are available to use with Auto Settings
  • Tweaks how we auto expose the Rcon (now RconAPI) endpoints to account for some edge cases
  • Standardizes some of the older endpoint names that didn't fit our current patterns (do_blacklist_player instead of blacklist_player) for instance
    • I kept the old endpoints for now, but would like to remove them in a few releases so we have less clutter
  • Updates the UI to use the new endpoint names
  • Updates the get_api_documentation endpoint to explicitly show which endpoints are usable with Auto Settings
  • Updates Auto Settings to allow partial updates for any user config settings, i.e. instead of passing the entire AutoModSeedingUserConfig you can pass a single (or multiple) keys like enabled and it will merge them with the previous values
  • Adds some miscellaneous typing
  • Removes some miscellaneous console log stuff that was in the UI
  • Removes a couple of miscellaneous endpoints that weren't (or ever?) exposed so they weren't usable and weren't used by anything
  • Update the discord audit function to allow optional escaping of markdown as it was escaping the automatically exposed Rcon endpoints
  • Updates remove_flag to allow removing a flag if a specific player has it (by the string value of the flag) instead of only supporting passing in the primary key of the flag recorsd
  • This carves out some exceptions to the POST and GET requirements that were being done by endpoint name, for instance the get_recent_logs endpoint has a lot of parameters and is traditionally passed via JSON

I believe (but I have not explicitly tested 100% of this manually) that I preserved the previous behaviors and argument names and I've been running this on our RCON for awhile now (two weeks maybe?) and have fixed all of the problems I've been able to find with it.

Old versus new API documentation: https://www.diffchecker.com/Km3MOLEV/

cemathey and others added 30 commits January 3, 2024 15:02
…ll_rcon_methods' into feat/autosettings_all_rcon_methods
@timraay
Copy link
Collaborator

timraay commented Jun 4, 2024

Here's one issue I ran into: http://localhost:8010/api/get_players_history?page_size=5&page=1

{
  "result": null,
  "command": "/api/get_players_history",
  "arguments": null,
  "failed": true,
  "error": "TypeError(\"'<=' not supported between instances of 'str' and 'int'\")",
  "forwards_results": null,
  "version": "v9.9.0-127-gb05feb5"
}

The error originates from a simple assertion that checks if the values fall within a certain range.

def get_players_by_appearance(
page: int = 1,
page_size: int = 500,
last_seen_from: datetime.datetime | None = None,
last_seen_till: datetime.datetime | None = None,
player_id: str | None = None,
player_name: str | None = None,
blacklisted: bool | None = None,
is_watched: bool | None = None,
exact_name_match: bool = False,
ignore_accent: bool = True,
flags: str | list[str] | None = None,
country: str | None = None,
):
if page <= 0:
raise ValueError("page needs to be >= 1")
if page_size <= 0:
raise ValueError("page_size needs to be >= 1")

The RconAPI method itself does specify they're ints in their signature though; it just does not seem to convert the input at any point.

def get_players_history(
self,
page: int = 1,
page_size: int = 500,
last_seen_from: datetime | None = None,
last_seen_till: datetime | None = None,

@cemathey cemathey mentioned this pull request Aug 20, 2024
@cemathey cemathey closed this Aug 20, 2024
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.

3 participants