-
Notifications
You must be signed in to change notification settings - Fork 38
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
more filters when requesting game metas #524
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some minor cosmetic things
heltour/tournament/lichessapi.py
Outdated
def get_latest_game_metas(lichess_username, number, priority=0, max_retries=5, timeout=1800): | ||
url = '%s/lichessapi/api/games/user/%s?max=%s&ongoing=true&priority=%s&max_retries=%s&format=application/x-ndjson' % ( | ||
settings.API_WORKER_HOST, lichess_username, number, priority, max_retries) | ||
def get_latest_game_metas(lichess_username, since, number, opponent, priority=0, max_retries=5, timeout=1800): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def get_latest_game_metas(lichess_username, since, number, opponent, priority=0, max_retries=5, timeout=1800): | |
def get_latest_game_metas(*, lichess_username, since, number, opponent, priority=0, max_retries=5, timeout=1800): |
positional arguments are the devil, use *
to enforce keyword arguments and make your code more readable/less error prone!
heltour/tournament/lichessapi.py
Outdated
settings.API_WORKER_HOST, lichess_username, number, priority, max_retries) | ||
def get_latest_game_metas(lichess_username, since, number, opponent, priority=0, max_retries=5, timeout=1800): | ||
url = '%s/lichessapi/api/games/user/%s?since=%s&max=%s&vs=%s&ongoing=true&priority=%s&max_retries=%s&format=application/x-ndjson' % ( | ||
settings.API_WORKER_HOST, lichess_username, since, number, opponent, priority, max_retries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use .format()
or preferable a format string f"{settings.API_WORKER_HOST}/lichessapi..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f7c8e7f
to
f838b52
Compare
implemented nic's comments and rebased from branch django_update |
closes #498. hopefully.
for games that are supposed to start soon, we requested the last games of the white player from lichess. we now filter these to only request games since the round started + only against the black player.
while i am still not certain how the bug above worked exactly, i assume that this should fix it. otherwise, i am out of ideas.
edit: rebased on #527
more edits: i believe this closes #255 as well.
edit to the edits: closes #202 too.