Skip to content

Commit 9f2d8b6

Browse files
landonshumway-ialandonshumway-ia
committed
PR feedback
1 parent 7ac9940 commit 9f2d8b6

2 files changed

Lines changed: 24 additions & 43 deletions

File tree

‎backend/compact-connect/lambdas/python/feature-flag/feature_flag_client.py‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,9 @@ def _make_console_api_request(
332332
if method == 'GET':
333333
response = requests.get(url, headers=headers, timeout=30)
334334
elif method == 'POST':
335-
response = requests.post(url, headers=headers, data=json.dumps(data), timeout=30)
335+
response = requests.post(url, headers=headers, json=data, timeout=30)
336336
elif method == 'PATCH':
337-
response = requests.patch(url, headers=headers, data=json.dumps(data), timeout=30)
337+
response = requests.patch(url, headers=headers, json=data, timeout=30)
338338
elif method == 'DELETE':
339339
response = requests.delete(url, headers=headers, timeout=30)
340340
else:
@@ -376,8 +376,7 @@ def upsert_flag(
376376
# we only set the environment rule if it doesn't already exist
377377
# else we leave it alone to avoid overwriting manual changes
378378
if not environment_rule:
379-
updated_gate = self._add_environment_rule(existing_gate, auto_enable, custom_attributes)
380-
self._update_gate(gate_id, updated_gate)
379+
self._add_environment_rule(gate_id, existing_gate, auto_enable, custom_attributes)
381380

382381
return existing_gate
383382

@@ -457,8 +456,8 @@ def _build_conditions_from_attributes(self, custom_attributes: dict[str, Any]) -
457456
return conditions
458457

459458
def _add_environment_rule(
460-
self, gate_data: dict[str, Any], auto_enable: bool, custom_attributes: dict[str, Any] | None = None
461-
) -> dict[str, Any]:
459+
self, gate_id: str, gate_data: dict[str, Any], auto_enable: bool, custom_attributes: dict[str, Any] | None = None
460+
) -> None:
462461
"""
463462
Add an environment-specific rule to an existing gate.
464463
@@ -489,7 +488,7 @@ def _add_environment_rule(
489488
updated_gate['rules'] = []
490489
updated_gate['rules'].append(new_rule)
491490

492-
return updated_gate
491+
self._update_gate(gate_id, updated_gate)
493492

494493
def _update_gate(self, gate_id: str, gate_data: dict[str, Any]) -> bool:
495494
"""

‎backend/compact-connect/lambdas/python/feature-flag/tests/function/test_statsig_client.py‎

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,7 @@ def test_upsert_flag_create_new_in_test_environment(self, mock_requests, mock_st
292292
'STATSIG-API-VERSION': STATSIG_API_VERSION,
293293
'Content-Type': 'application/json',
294294
},
295-
data=json.dumps(
296-
{
295+
json={
297296
'name': 'new-test-flag',
298297
'description': 'Feature gate managed by CDK for new-test-flag feature',
299298
'isEnabled': True,
@@ -312,8 +311,7 @@ def test_upsert_flag_create_new_in_test_environment(self, mock_requests, mock_st
312311
'passPercentage': 100, # Always 100 for test environment
313312
}
314313
],
315-
}
316-
),
314+
},
317315
timeout=30,
318316
)
319317

@@ -346,8 +344,7 @@ def test_upsert_flag_create_new_in_test_environment_no_attributes(self, mock_req
346344
'STATSIG-API-VERSION': STATSIG_API_VERSION,
347345
'Content-Type': 'application/json',
348346
},
349-
data=json.dumps(
350-
{
347+
json={
351348
'name': 'simple-flag',
352349
'description': 'Feature gate managed by CDK for simple-flag feature',
353350
'isEnabled': True,
@@ -359,8 +356,7 @@ def test_upsert_flag_create_new_in_test_environment_no_attributes(self, mock_req
359356
'passPercentage': 0,
360357
}
361358
],
362-
}
363-
),
359+
},
364360
timeout=30,
365361
)
366362

@@ -428,8 +424,7 @@ def test_upsert_flag_prod_environment_auto_enable_false_no_existing_flag(self, m
428424
'STATSIG-API-VERSION': STATSIG_API_VERSION,
429425
'Content-Type': 'application/json',
430426
},
431-
data=json.dumps(
432-
{
427+
json={
433428
'name': 'prod-flag',
434429
'description': 'Feature gate managed by CDK for prod-flag feature',
435430
'isEnabled': True,
@@ -441,8 +436,7 @@ def test_upsert_flag_prod_environment_auto_enable_false_no_existing_flag(self, m
441436
'passPercentage': 0, # Disabled in prod when auto_enable=False
442437
}
443438
],
444-
}
445-
),
439+
},
446440
timeout=30,
447441
)
448442

@@ -475,8 +469,7 @@ def test_upsert_flag_prod_environment_auto_enable_true_no_existing_flag(self, mo
475469
'STATSIG-API-VERSION': STATSIG_API_VERSION,
476470
'Content-Type': 'application/json',
477471
},
478-
data=json.dumps(
479-
{
472+
json={
480473
'name': 'prod-flag',
481474
'description': 'Feature gate managed by CDK for prod-flag feature',
482475
'isEnabled': True,
@@ -488,8 +481,7 @@ def test_upsert_flag_prod_environment_auto_enable_true_no_existing_flag(self, mo
488481
'passPercentage': 100,
489482
}
490483
],
491-
}
492-
),
484+
},
493485
timeout=30,
494486
)
495487

@@ -535,8 +527,7 @@ def test_upsert_flag_beta_environment_auto_enable_false_no_existing_rule_create_
535527
'STATSIG-API-VERSION': STATSIG_API_VERSION,
536528
'Content-Type': 'application/json',
537529
},
538-
data=json.dumps(
539-
{
530+
json={
540531
'id': 'existing-flag',
541532
'name': 'existing-flag',
542533
'rules': [
@@ -553,8 +544,7 @@ def test_upsert_flag_beta_environment_auto_enable_false_no_existing_rule_create_
553544
'passPercentage': 0,
554545
},
555546
],
556-
}
557-
),
547+
},
558548
timeout=30,
559549
)
560550

@@ -601,8 +591,7 @@ def test_upsert_flag_prod_environment_existing_flag_auto_enable_true(self, mock_
601591
'STATSIG-API-VERSION': STATSIG_API_VERSION,
602592
'Content-Type': 'application/json',
603593
},
604-
data=json.dumps(
605-
{
594+
json={
606595
'id': 'gate-existing-prod',
607596
'name': 'existing-prod-flag',
608597
'rules': [
@@ -626,8 +615,7 @@ def test_upsert_flag_prod_environment_existing_flag_auto_enable_true(self, mock_
626615
'passPercentage': 100,
627616
},
628617
],
629-
}
630-
),
618+
},
631619
timeout=30,
632620
)
633621

@@ -851,8 +839,7 @@ def test_delete_flag_multiple_rules_removes_current_rule_only(self, mock_request
851839
'STATSIG-API-VERSION': STATSIG_API_VERSION,
852840
'Content-Type': 'application/json',
853841
},
854-
data=json.dumps(
855-
{
842+
json={
856843
'id': 'gate-delete-multi',
857844
'name': 'delete-multi-flag',
858845
'rules': [
@@ -869,8 +856,7 @@ def test_delete_flag_multiple_rules_removes_current_rule_only(self, mock_request
869856
'passPercentage': 100,
870857
},
871858
],
872-
}
873-
),
859+
},
874860
timeout=30,
875861
)
876862
mock_requests.delete.assert_not_called()
@@ -1059,8 +1045,7 @@ def test_upsert_flag_custom_attributes_as_string(self, mock_requests, mock_stats
10591045
'STATSIG-API-VERSION': STATSIG_API_VERSION,
10601046
'Content-Type': 'application/json',
10611047
},
1062-
data=json.dumps(
1063-
{
1048+
json={
10641049
'name': 'string-attrs-flag',
10651050
'description': 'Feature gate managed by CDK for string-attrs-flag feature',
10661051
'isEnabled': True,
@@ -1080,8 +1065,7 @@ def test_upsert_flag_custom_attributes_as_string(self, mock_requests, mock_stats
10801065
'passPercentage': 0, # Always 100 for test environment
10811066
}
10821067
],
1083-
}
1084-
),
1068+
},
10851069
timeout=30,
10861070
)
10871071

@@ -1115,8 +1099,7 @@ def test_upsert_flag_custom_attributes_as_list(self, mock_requests, mock_statsig
11151099
'STATSIG-API-VERSION': STATSIG_API_VERSION,
11161100
'Content-Type': 'application/json',
11171101
},
1118-
data=json.dumps(
1119-
{
1102+
json={
11201103
'name': 'list-attrs-flag',
11211104
'description': 'Feature gate managed by CDK for list-attrs-flag feature',
11221105
'isEnabled': True,
@@ -1135,8 +1118,7 @@ def test_upsert_flag_custom_attributes_as_list(self, mock_requests, mock_statsig
11351118
'passPercentage': 0,
11361119
}
11371120
],
1138-
}
1139-
),
1121+
},
11401122
timeout=30,
11411123
)
11421124

0 commit comments

Comments
 (0)