Refactor: move Python SDK constants to a dedicated file and add PodSnapshot constants#337
Refactor: move Python SDK constants to a dedicated file and add PodSnapshot constants#337shrutiyam-glitch wants to merge 2 commits intokubernetes-sigs:mainfrom
Conversation
✅ Deploy Preview for agent-sandbox canceled.
|
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @shrutiyam-glitch. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
size/M
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| logging.basicConfig(level=logging.INFO, | ||
| format='%(asctime)s - %(levelname)s - %(message)s', | ||
| stream=sys.stdout) | ||
| from .constants import * |
There was a problem hiding this comment.
Avoid wildcard imports as it pollutes the namespace and makes it difficult to track where names are defined. It is more idiomatic to use explicit imports or from . import constants.
| @@ -0,0 +1,37 @@ | |||
| # Copyright 2025 The Kubernetes Authors. | |||
| PODSNAPSHOT_API_VERSION = "v1alpha1" | ||
| PODSNAPSHOT_PLURAL = "podsnapshots" | ||
| PODSNAPSHOTMANUALTRIGGER_PLURAL = "podsnapshotmanualtriggers" | ||
| PODSNAPSHOT_API_KIND = "PodSnapshotManualTrigger" |
There was a problem hiding this comment.
These constants are defined but currently unused in the client.
A PR should focus on doing one thing. A good way to tell is when you need to add "and" in your commit message, you should split the PR/commit.
You should also only add things when they're being used, so that we make sure the code is clean and won't have zombie code that were added at some point but never used because of a change of plan.
There was a problem hiding this comment.
Thanks. I will remove the PodSnapshot constants for now to keep this PR focused strictly on the refactoring of existing constants. I'll re-introduce them in a follow-up PR when the implementation that actually uses them is ready.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shrutiyam-glitch The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
3 participants
This PR refactors the Python SDK by extracting API-related constants into a new
constants.pyfile and adding definitions for thePodSnapshot API.Changes:
constants.py: Centralized API group, version, and resource constants for better maintainability.sandbox_client.py: Removed inline constant definitions and replaced them with a wildcard import from the new constants module to keep the client code clean.Why this is needed:
Moving constants to a dedicated file improves code organization, reduces duplication, and makes these values easily reusable across other components of the python package.