Skip to content

Commit d928e0a

Browse files
committed
BUG: Equality Vs Identity
Fix a number of instances where Equality was being checked over Identity. While no bug currently existed, this pattern was could have caused an extremely hard to track down bug. ```python >>> 1 == True # Equality True >>> 1 is True # Identity False ``` Signed-off-by: Jim Fitzpatrick <jfitzpat@redhat.com>
1 parent e8404ec commit d928e0a

File tree

14 files changed

+50
-62
lines changed

14 files changed

+50
-62
lines changed

src/codeflare_sdk/common/kubernetes_cluster/auth.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def load_kube_config(self):
151151
global config_path
152152
global api_client
153153
try:
154-
if self.kube_config_path == None:
154+
if self.kube_config_path is None:
155155
return "Please specify a config file path"
156156
config_path = self.kube_config_path
157157
api_client = None
@@ -183,7 +183,7 @@ def config_check() -> str:
183183
global config_path
184184
global api_client
185185
home_directory = os.path.expanduser("~")
186-
if config_path == None and api_client == None:
186+
if config_path is None and api_client is None:
187187
if os.path.isfile("%s/.kube/config" % home_directory):
188188
try:
189189
config.load_kube_config()
@@ -199,7 +199,7 @@ def config_check() -> str:
199199
"Action not permitted, have you put in correct/up-to-date auth credentials?"
200200
)
201201

202-
if config_path != None and api_client == None:
202+
if config_path is not None and api_client is None:
203203
return config_path
204204

205205

@@ -237,7 +237,7 @@ def get_api_client() -> client.ApiClient:
237237
client.ApiClient:
238238
The Kubernetes API client object.
239239
"""
240-
if api_client != None:
240+
if api_client is not None:
241241
return api_client
242242
to_return = client.ApiClient()
243243
_client_with_cert(to_return)

src/codeflare_sdk/common/kubernetes_cluster/test_auth.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,20 @@ def test_token_auth_creation():
3030
token_auth = TokenAuthentication(token="token", server="server")
3131
assert token_auth.token == "token"
3232
assert token_auth.server == "server"
33-
assert token_auth.skip_tls == False
34-
assert token_auth.ca_cert_path == None
33+
assert token_auth.skip_tls is False
34+
assert token_auth.ca_cert_path is None
3535

3636
token_auth = TokenAuthentication(token="token", server="server", skip_tls=True)
3737
assert token_auth.token == "token"
3838
assert token_auth.server == "server"
39-
assert token_auth.skip_tls == True
40-
assert token_auth.ca_cert_path == None
39+
assert token_auth.skip_tls is True
40+
assert token_auth.ca_cert_path is None
4141

4242
os.environ["CF_SDK_CA_CERT_PATH"] = "/etc/pki/tls/custom-certs/ca-bundle.crt"
4343
token_auth = TokenAuthentication(token="token", server="server", skip_tls=False)
4444
assert token_auth.token == "token"
4545
assert token_auth.server == "server"
46-
assert token_auth.skip_tls == False
46+
assert token_auth.skip_tls is False
4747
assert token_auth.ca_cert_path == "/etc/pki/tls/custom-certs/ca-bundle.crt"
4848
os.environ.pop("CF_SDK_CA_CERT_PATH")
4949

@@ -55,7 +55,7 @@ def test_token_auth_creation():
5555
)
5656
assert token_auth.token == "token"
5757
assert token_auth.server == "server"
58-
assert token_auth.skip_tls == False
58+
assert token_auth.skip_tls is False
5959
assert token_auth.ca_cert_path == f"{parent}/tests/auth-test.crt"
6060

6161

@@ -116,7 +116,7 @@ def test_config_check_with_incluster_config(mocker):
116116
mocker.patch("codeflare_sdk.common.kubernetes_cluster.auth.api_client", None)
117117

118118
result = config_check()
119-
assert result == None
119+
assert result is None
120120

121121

122122
def test_config_check_with_existing_config_file(mocker):
@@ -127,7 +127,7 @@ def test_config_check_with_existing_config_file(mocker):
127127
mocker.patch("codeflare_sdk.common.kubernetes_cluster.auth.api_client", None)
128128

129129
result = config_check()
130-
assert result == None
130+
assert result is None
131131

132132

133133
def test_config_check_with_config_path_and_no_api_client(mocker):

src/codeflare_sdk/common/kueue/kueue.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ def add_queue_label(item: dict, namespace: str, local_queue: Optional[str]):
163163
If the provided or default local queue does not exist in the namespace.
164164
"""
165165
lq_name = local_queue or get_default_kueue_name(namespace)
166-
if lq_name == None:
166+
if lq_name is None:
167167
return
168168
elif not local_queue_exists(namespace, lq_name):
169169
raise ValueError(

src/codeflare_sdk/common/kueue/test_kueue.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def test_none_local_queue(mocker):
3636
config.local_queue = None
3737

3838
cluster = Cluster(config)
39-
assert cluster.config.local_queue == None
39+
assert cluster.config.local_queue is None
4040

4141

4242
def test_cluster_creation_no_aw_local_queue(mocker):

src/codeflare_sdk/common/utils/test_generate_cert.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ def test_generate_ca_cert():
4343
cert_pub_key_bytes = cert.public_key().public_bytes(
4444
Encoding.PEM, PublicFormat.SubjectPublicKeyInfo
4545
)
46-
assert type(key) == str
47-
assert type(certificate) == str
46+
assert isinstance(key, str)
47+
assert isinstance(certificate, str)
4848
# Veirfy ca.cert is self signed
49-
assert cert.verify_directly_issued_by(cert) == None
49+
assert cert.verify_directly_issued_by(cert) is None
5050
# Verify cert has the public key bytes from the private key
5151
assert cert_pub_key_bytes == private_pub_key_bytes
5252

@@ -84,7 +84,7 @@ def test_generate_tls_cert(mocker):
8484
tls_cert = load_pem_x509_certificate(f.read().encode("utf-8"))
8585
with open(os.path.join("tls-cluster-namespace", "ca.crt"), "r") as f:
8686
root_cert = load_pem_x509_certificate(f.read().encode("utf-8"))
87-
assert tls_cert.verify_directly_issued_by(root_cert) == None
87+
assert tls_cert.verify_directly_issued_by(root_cert) is None
8888

8989

9090
def test_export_env():

src/codeflare_sdk/ray/appwrapper/test_awload.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ def test_AWManager_creation(mocker):
4242
testaw = AWManager(f"{aw_dir}test.yaml")
4343
assert testaw.name == "test"
4444
assert testaw.namespace == "ns"
45-
assert testaw.submitted == False
45+
assert testaw.submitted is False
4646
try:
4747
testaw = AWManager("fake")
4848
except Exception as e:
49-
assert type(e) == FileNotFoundError
49+
assert isinstance(e, FileNotFoundError)
5050
assert str(e) == "[Errno 2] No such file or directory: 'fake'"
5151
try:
5252
testaw = apply_template(
@@ -56,7 +56,7 @@ def test_AWManager_creation(mocker):
5656
get_template_variables(),
5757
)
5858
except Exception as e:
59-
assert type(e) == ValueError
59+
assert isinstance(e, ValueError)
6060
assert (
6161
str(e)
6262
== f"{parent}/tests/test_cluster_yamls/appwrapper/test-case-bad.yaml is not a correctly formatted AppWrapper yaml"
@@ -72,7 +72,7 @@ def test_AWManager_submit_remove(mocker, capsys):
7272
captured.out
7373
== "AppWrapper not submitted by this manager yet, nothing to remove\n"
7474
)
75-
assert testaw.submitted == False
75+
assert testaw.submitted is False
7676
mocker.patch("kubernetes.config.load_kube_config", return_value="ignore")
7777
mocker.patch(
7878
"kubernetes.client.CustomObjectsApi.create_namespaced_custom_object",
@@ -83,9 +83,9 @@ def test_AWManager_submit_remove(mocker, capsys):
8383
side_effect=arg_check_aw_del_effect,
8484
)
8585
testaw.submit()
86-
assert testaw.submitted == True
86+
assert testaw.submitted is True
8787
testaw.remove()
88-
assert testaw.submitted == False
88+
assert testaw.submitted is False
8989

9090

9191
# Make sure to always keep this function last

src/codeflare_sdk/ray/appwrapper/test_status.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,34 +51,34 @@ def test_cluster_status(mocker):
5151
)
5252
status, ready = cf.status()
5353
assert status == CodeFlareClusterStatus.UNKNOWN
54-
assert ready == False
54+
assert ready is False
5555

5656
mocker.patch(
5757
"codeflare_sdk.ray.cluster.cluster._app_wrapper_status", return_value=fake_aw
5858
)
5959
status, ready = cf.status()
6060
assert status == CodeFlareClusterStatus.FAILED
61-
assert ready == False
61+
assert ready is False
6262

6363
fake_aw.status = AppWrapperStatus.SUSPENDED
6464
status, ready = cf.status()
6565
assert status == CodeFlareClusterStatus.QUEUED
66-
assert ready == False
66+
assert ready is False
6767

6868
fake_aw.status = AppWrapperStatus.RESUMING
6969
status, ready = cf.status()
7070
assert status == CodeFlareClusterStatus.STARTING
71-
assert ready == False
71+
assert ready is False
7272

7373
fake_aw.status = AppWrapperStatus.RESETTING
7474
status, ready = cf.status()
7575
assert status == CodeFlareClusterStatus.STARTING
76-
assert ready == False
76+
assert ready is False
7777

7878
fake_aw.status = AppWrapperStatus.RUNNING
7979
status, ready = cf.status()
8080
assert status == CodeFlareClusterStatus.UNKNOWN
81-
assert ready == False
81+
assert ready is False
8282

8383

8484
def aw_status_fields(group, version, namespace, plural, *args):
@@ -97,7 +97,7 @@ def test_aw_status(mocker):
9797
side_effect=aw_status_fields,
9898
)
9999
aw = _app_wrapper_status("test-aw", "test-ns")
100-
assert aw == None
100+
assert aw is None
101101

102102

103103
# Make sure to always keep this function last

src/codeflare_sdk/ray/cluster/build_ray_cluster.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ def add_queue_label(cluster: "codeflare_sdk.ray.cluster.Cluster", labels: dict):
482482
The add_queue_label() function updates the given base labels with the local queue label if Kueue exists on the Cluster
483483
"""
484484
lq_name = cluster.config.local_queue or get_default_local_queue(cluster, labels)
485-
if lq_name == None:
485+
if lq_name is None:
486486
return
487487
elif not local_queue_exists(cluster):
488488
# ValueError removed to pass validation to validating admission policy

src/codeflare_sdk/ray/cluster/cluster.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ def cluster_dashboard_uri(self) -> str:
513513
ingress.metadata.name == f"ray-dashboard-{self.config.name}"
514514
or ingress.metadata.name.startswith(f"{self.config.name}-ingress")
515515
):
516-
if annotations == None:
516+
if annotations is None:
517517
protocol = "http"
518518
elif "route.openshift.io/termination" in annotations:
519519
protocol = "https"
@@ -865,7 +865,7 @@ def _check_aw_exists(name: str, namespace: str) -> bool:
865865
def _get_ingress_domain(self): # pragma: no cover
866866
config_check()
867867

868-
if self.config.namespace != None:
868+
if self.config.namespace is not None:
869869
namespace = self.config.namespace
870870
else:
871871
namespace = get_current_namespace()
@@ -1037,7 +1037,7 @@ def _map_to_ray_cluster(rc) -> Optional[RayCluster]:
10371037
ingress.metadata.name == f"ray-dashboard-{rc['metadata']['name']}"
10381038
or ingress.metadata.name.startswith(f"{rc['metadata']['name']}-ingress")
10391039
):
1040-
if annotations == None:
1040+
if annotations is None:
10411041
protocol = "http"
10421042
elif "route.openshift.io/termination" in annotations:
10431043
protocol = "https"

src/codeflare_sdk/ray/cluster/test_cluster.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ def test_wait_ready(mocker, capsys):
534534
cf.wait_ready(timeout=5)
535535
assert 1 == 0
536536
except Exception as e:
537-
assert type(e) == TimeoutError
537+
assert isinstance(e, TimeoutError)
538538

539539
captured = capsys.readouterr()
540540
assert (
@@ -606,7 +606,7 @@ def test_list_queue_rayclusters(mocker, capsys):
606606
]
607607
mocker.patch("kubernetes.client.ApisApi", return_value=mock_api)
608608

609-
assert _is_openshift_cluster() == True
609+
assert _is_openshift_cluster() is True
610610
mocker.patch(
611611
"kubernetes.client.CustomObjectsApi.list_namespaced_custom_object",
612612
return_value=get_obj_none("ray.io", "v1", "ns", "rayclusters"),

0 commit comments

Comments
 (0)