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

Upgrade to use Challonge API v2.1 #723

Merged
merged 6 commits into from
Sep 28, 2024
Merged

Upgrade to use Challonge API v2.1 #723

merged 6 commits into from
Sep 28, 2024

Conversation

n8kim1
Copy link
Contributor

@n8kim1 n8kim1 commented Jan 15, 2024

No description provided.

@@ -25,7 +25,7 @@
}

AUTH_TYPE = "v1"
Copy link
Member

Choose a reason for hiding this comment

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

What's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Challonge's authentication version
v1 is with keys, v2 is the weird OAuth/MFA stuff that we couldn't figure out last year

Copy link
Member

Choose a reason for hiding this comment

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

seems unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh good catch

for (siarnaq_player_index, challonge_player_index) in enumerate(
["player1", "player2"]
):
for player_index in range(2):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for player_index in range(2):
for (player_index, challonge_participant) in enumerate(
challonge_match["attributes"]["points_by_participant"]
):

removes magic number 2 by lifting stuff up from l.327

Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

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

Have we tested bulk add teams endpoint? specifically the payload data.type being "Participant" instead of "Participants" like in the challonge docs
image
https://api.challonge.com/v2/api_docs/single_swagger_doc#!/Participant/bulkCreateParticipant

Also start_tournament()'s payload.data.type being "TournamentState" and payload.data.attributes.state don't seem to exist in challonge's docs

Is this just their documentation being incomplete / have you tested these functions?

@n8kim1
Copy link
Contributor Author

n8kim1 commented Jan 20, 2024 via email

@n8kim1
Copy link
Contributor Author

n8kim1 commented Jan 20, 2024 via email

@acrantel
Copy link
Member

acrantel commented Jan 21, 2024 via email

@n8kim1 n8kim1 requested review from acrantel and removed request for acrantel and lowtorola May 27, 2024 01:41
@n8kim1
Copy link
Contributor Author

n8kim1 commented May 27, 2024

yes its summer and yes nathan is still working on this, sorry... I'm just cleaning up some easy and quick loose ends

#723 (review)
Bulk add endpoint is fixed! Thanks for the catch, idk how it slipped testing
Tournament state is in the docs (although kinda hard to spot) -- testing that and it works

@n8kim1 n8kim1 requested review from acrantel and lowtorola May 27, 2024 02:07
Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

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

lgtm pt 2 :shipit:

@n8kim1 n8kim1 merged commit 6ca5377 into main Sep 28, 2024
3 checks passed
@n8kim1 n8kim1 deleted the nkim-tour-api-v2.1 branch September 28, 2024 23:42
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