fix(package): Add StatusIntEnum class to match the behavior of IntEnum in Python 3.11.#1133
Conversation
WalkthroughA new base class Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: user haiqi96 requested creating a github issue to document a bug fix from pr #1136, which addressed ...Applied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/job-orchestration/job_orchestration/scheduler/constants.py (2)
31-36: Same duplication observation as above – refer to the earlier comment for the proposed mix-in approach.
44-49: Same duplication observation as above – refer to the earlier comment for the proposed mix-in approach.
junhaoliao
left a comment
There was a problem hiding this comment.
On ec2's arm machine, without those methods, python convert enums to int instead of string and has caused failures when updating the database.
Mind giving a code example of how the enum is converted?
one example can be found here Inside update_compression_job_metadata(), the compression_job_status enum will be converted, I guess the str method will be implicitly called. |
__str()__ method for job orchestration enums for mysql support.
__str()__ method for job orchestration enums for mysql support.__str()__ method for job orchestration enums for MySQL compatibility.
__str()__ method for job orchestration enums for MySQL compatibility.__str()__ and to_str() method for job orchestration enums for MySQL compatibility.
There was a problem hiding this comment.
instead of overriding individual classes' __str__ method, can we add a mixin like
class IntEnumStrMixin:
"""
Delegates __str__ to int.__str__, matching the behavior of IntEnum
in Python 3.11+.
TODO: Remove this when our minimum supported Python version is 3.11+.
"""
def __str__(self):
return int.__str__(self.__getattribute__("value"))
class CompressionJobStatus(IntEnumStrMixin, IntEnum):
PENDING = 0
RUNNING = auto()
SUCCEEDED = auto()
FAILED = auto()alternatively, we can also extend the IntEnum class into a custom class and let the existing enum classes extend from it.
There was a problem hiding this comment.
Updated my code. let me know if you agree with the current approach. If you don't see any issues, I will then update the PR description.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/job-orchestration/job_orchestration/scheduler/constants.py(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.
📚 Learning: user haiqi96 requested creating a github issue to document a bug fix from pr #1136, which addressed ...
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.
Applied to files:
components/job-orchestration/job_orchestration/scheduler/constants.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint-check (macos-15)
- GitHub Check: build (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (1)
components/job-orchestration/job_orchestration/scheduler/constants.py (1)
14-21: Centralizing__str__/to_strlogic viaStatusIntEnumcleans up the module nicelyMoving the duplicated methods into a single base class addresses the earlier DRY feedback and keeps future enum additions painless. 👍
junhaoliao
left a comment
There was a problem hiding this comment.
for the title, how about
fix(package): Add `__str()__` and `to_str()` methods to job orchestration enums for MySQL connector compatibility.
I will think about the title since now I am introducing a new class. Will let you know when I finish updating the PR. |
__str()__ and to_str() method for job orchestration enums for MySQL compatibility.StatusIntEnum class to match behavior the behavior of IntEnum in python 3.11.
StatusIntEnum class to match behavior the behavior of IntEnum in python 3.11.StatusIntEnum class to match the behavior of IntEnum in python 3.11.
junhaoliao
left a comment
There was a problem hiding this comment.
For the title, let's capitalize the "P" in "Python"
StatusIntEnum class to match the behavior of IntEnum in python 3.11.StatusIntEnum class to match the behavior of IntEnum in Python 3.11.
…Enum` in Python 3.11. (y-scope#1133)
Description
This PR adds
StatusIntEnumwhich derived onIntEnumso that__str__'s behavior is aligned with python 3.11, as documented in hereIn our code, we currently rely on the database connector to implicilty convert the scheduler enums when writting them to the database.
Note that when using mysql, the library (likely
mysql-connector-python) converts the enum using str(). With a lower python version, the enum will be converted to string and cause an error.As a sollution, this PR introduce
StatusIntEnumwhich overloads the str method, and let all status related enum to derive onStatusIntEnum.To reproduce, change the database type to "mysql" and compress any input logs. the compression scheduler will fail with error log:
Note the same issue is not observed when using mariadb, which could be internally handled by mariadb's python library.
Checklist
breaking change.
Validation performed
Configured database.type = mysql
Compressed mongodb log from local file storage, ensured the compression scheduler doesn't crash
Summary by CodeRabbit
Summary by CodeRabbit