Skip to content

Comments

Fix: remove tzinfo#348

Merged
S1ro1 merged 1 commit intomainfrom
fix/tzinfo
Aug 31, 2025
Merged

Fix: remove tzinfo#348
S1ro1 merged 1 commit intomainfrom
fix/tzinfo

Conversation

@S1ro1
Copy link
Member

@S1ro1 S1ro1 commented Aug 31, 2025

Description

Please provide a brief summary of the changes in this pull request.

Checklist

Before submitting this PR, ensure the following steps have been completed:

  • Run the slash command /verifyruns on your own server.
    • Run the cluster bot on your server:
      python discord-bot.py
    • Start training runs with the slash command /verifyruns.
    • Verify that the bot eventually responds with:
      ✅ All runs completed successfully!
      
      (It may take a few minutes for all runs to finish. In particular, the GitHub
      runs may take a little longer. The Modal run is typically quick.)
      For more information on running a cluster bot on your own server, see
      README.md.

Copilot AI review requested due to automatic review settings August 31, 2025 19:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes timezone validation and cleans up formatting in the leaderboard database module. The main change removes an assertion that required deadline timestamps to have timezone information.

  • Remove timezone validation assertion for deadline parameter
  • Reformat return statement for better code style
  • Clean up extra whitespace

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 81 to 83
gpu_types: list | str,
) -> int:
# to prevent surprises, ensure we have specified a timezone
Copy link

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

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

The comment on line 83 mentions ensuring a timezone is specified, but the corresponding assertion has been removed. This comment should be updated or removed to avoid confusion.

Suggested change
gpu_types: list | str,
) -> int:
# to prevent surprises, ensure we have specified a timezone

Copilot uses AI. Check for mistakes.
Comment on lines +262 to +264
return (
{"user_id": row[0], "user_name": row[1], "id_type": id_type.value} if row else None
)
Copy link

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

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

[nitpick] The parentheses around the return statement are unnecessary. The conditional expression can be returned directly without wrapping parentheses.

Suggested change
return (
{"user_id": row[0], "user_name": row[1], "id_type": id_type.value} if row else None
)
return {"user_id": row[0], "user_name": row[1], "id_type": id_type.value} if row else None

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/libkernelbot
  leaderboard_db.py
Project Total  

This report was generated by python-coverage-comment-action

@S1ro1 S1ro1 merged commit f8f0460 into main Aug 31, 2025
5 checks passed
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.

2 participants