Skip to content

Commit 66f8041

Browse files
committed
Logging improvement, fix tests
1 parent 7fa15c1 commit 66f8041

File tree

3 files changed

+55
-14
lines changed

3 files changed

+55
-14
lines changed

src/redis_release/bht/behaviours.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ def update(self) -> Status:
298298
if self.log_once(
299299
"workflow_triggered", self.workflow.ephemeral.log_once_flags
300300
):
301-
logger.info(
301+
self.logger.info(
302302
f"[green]Workflow triggered successfully:[/green] {self.workflow.uuid}"
303303
)
304304
self.feedback_message = "workflow triggered"
@@ -480,6 +480,9 @@ def initialise(self) -> None:
480480
)
481481
return
482482

483+
self.logger.info(
484+
f"Start getting artifacts for workflow {self.workflow.workflow_file}, run_id: {self.workflow.run_id}"
485+
)
483486
self.task = asyncio.create_task(
484487
self.github_client.get_workflow_artifacts(
485488
self.package_meta.repo, self.workflow.run_id
@@ -495,9 +498,12 @@ def update(self) -> Status:
495498

496499
result = self.task.result()
497500
self.workflow.artifacts = result
498-
self.logger.info(
499-
f"[green]Downloaded artifacts list:[/green] {len(result)} {result} artifacts"
500-
)
501+
if self.log_once(
502+
"workflow_artifacts_list", self.workflow.ephemeral.log_once_flags
503+
):
504+
self.logger.info(
505+
f"[green]Downloaded artifacts list:[/green] {len(result)} artifacts"
506+
)
501507
self.feedback_message = f"Downloaded {len(result)} artifacts"
502508
return Status.SUCCESS
503509
except Exception as e:
@@ -810,7 +816,7 @@ def __init__(self, name: str, workflow: Workflow, log_prefix: str = "") -> None:
810816
def update(self) -> Status:
811817
if self.workflow.artifacts is not None:
812818
if self.log_once(
813-
"workflow_artifacts", self.workflow.ephemeral.log_once_flags
819+
"workflow_artifacts_list", self.workflow.ephemeral.log_once_flags
814820
):
815821
self.logger.info(f"Workflow has artifacts")
816822
return Status.SUCCESS
@@ -825,7 +831,9 @@ def __init__(self, name: str, workflow: Workflow, log_prefix: str = "") -> None:
825831
def update(self) -> Status:
826832
if self.workflow.result is not None:
827833
if self.log_once("workflow_result", self.workflow.ephemeral.log_once_flags):
828-
self.logger.info(f"Workflow is successful and has result")
834+
self.logger.info(
835+
f"Workflow {self.workflow.workflow_file}, run_id: {self.workflow.run_id} is successful and has result"
836+
)
829837
return Status.SUCCESS
830838
return Status.FAILURE
831839

src/redis_release/github_client_async.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ async def trigger_workflow(
238238
Returns:
239239
WorkflowRun object with basic information (workflow identification will be done separately)
240240
"""
241-
logger.info(f"[blue]Triggering workflow[/blue] {workflow_file} in {repo}")
241+
logger.debug(f"[blue]Triggering workflow[/blue] {workflow_file} in {repo}")
242242
logger.debug(f"Inputs: {inputs}")
243243
logger.debug(f"Ref: {ref}")
244244
logger.debug(f"Workflow UUID: [cyan]{inputs['workflow_uuid']}[/cyan]")
@@ -264,7 +264,7 @@ async def trigger_workflow(
264264
timeout=30,
265265
error_context="trigger workflow",
266266
)
267-
logger.info(f"[green]Workflow triggered successfully[/green]")
267+
logger.debug(f"[green]Workflow triggered successfully[/green]")
268268
return True
269269
except aiohttp.ClientError as e:
270270
logger.error(f"[red]Failed to trigger workflow:[/red] {e}")
@@ -283,7 +283,9 @@ async def identify_workflow(
283283
for run in runs:
284284
extracted_uuid = self._extract_uuid(run.workflow_id)
285285
if extracted_uuid and extracted_uuid.lower() == workflow_uuid.lower():
286-
logger.info(f"[green]Found matching workflow run:[/green] {run.run_id}")
286+
logger.debug(
287+
f"[green]Found matching workflow run:[/green] {run.run_id}"
288+
)
287289
logger.debug(f"Workflow name: {run.workflow_id}")
288290
logger.debug(f"Extracted UUID: {extracted_uuid}")
289291
run.workflow_uuid = workflow_uuid
@@ -436,7 +438,7 @@ async def get_workflow_artifacts(self, repo: str, run_id: int) -> Dict[str, Dict
436438
Each artifact dictionary contains: id, archive_download_url, created_at,
437439
expires_at, updated_at, size_in_bytes, digest
438440
"""
439-
logger.info(f"[blue]Getting artifacts for workflow {run_id} in {repo}[/blue]")
441+
logger.debug(f"[blue]Getting artifacts for workflow {run_id} in {repo}[/blue]")
440442

441443
url = f"https://api.github.com/repos/{repo}/actions/runs/{run_id}/artifacts"
442444
headers = {
@@ -482,7 +484,7 @@ async def get_workflow_artifacts(self, repo: str, run_id: int) -> Dict[str, Dict
482484
artifacts[artifact_name] = artifact_info
483485

484486
if artifacts:
485-
logger.info(f"[green]Found {len(artifacts)} artifacts[/green]")
487+
logger.debug(f"[green]Found {len(artifacts)} artifacts[/green]")
486488
for artifact_name, artifact_info in artifacts.items():
487489
size_mb = round(
488490
artifact_info.get("size_in_bytes", 0) / (1024 * 1024), 2
@@ -533,7 +535,7 @@ async def download_and_extract_json_result(
533535
logger.error(f"[red]{artifact_name} artifact has no ID[/red]")
534536
return None
535537

536-
logger.info(
538+
logger.debug(
537539
f"[blue]Extracting {json_file_name} from artifact {artifact_id}[/blue]"
538540
)
539541

@@ -570,7 +572,7 @@ async def download_and_extract_json_result(
570572
if json_file_name in zip_file.namelist():
571573
with zip_file.open(json_file_name) as json_file_obj:
572574
result_data = json.load(json_file_obj)
573-
logger.info(
575+
logger.debug(
574576
f"[green]Successfully extracted {json_file_name}[/green]"
575577
)
576578
return result_data
@@ -643,7 +645,7 @@ async def list_remote_branches(
643645
if pattern:
644646
branches = [branch for branch in branches if re.match(pattern, branch)]
645647

646-
logger.info(
648+
logger.debug(
647649
f"[green]Found {len(branches)} branches{' matching pattern' if pattern else ''}[/green]"
648650
)
649651
if pattern:

src/tests/test_state.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
Workflow,
1414
)
1515
from redis_release.config import Config, PackageConfig
16+
from redis_release.models import PackageType
1617

1718

1819
class TestReleaseStateFromConfig:
@@ -26,6 +27,7 @@ def test_from_config_with_valid_workflows(self) -> None:
2627
packages={
2728
"test-package": PackageConfig(
2829
repo="test/repo",
30+
package_type=PackageType.DEBIAN,
2931
build_workflow="build.yml",
3032
publish_workflow="publish.yml",
3133
)
@@ -50,6 +52,7 @@ def test_from_config_with_custom_timeout_values(self) -> None:
5052
packages={
5153
"test-package": PackageConfig(
5254
repo="test/repo",
55+
package_type=PackageType.DEBIAN,
5356
build_workflow="build.yml",
5457
build_timeout_minutes=60,
5558
publish_workflow="publish.yml",
@@ -70,6 +73,7 @@ def test_from_config_with_ref(self) -> None:
7073
packages={
7174
"test-package": PackageConfig(
7275
repo="test/repo",
76+
package_type=PackageType.DEBIAN,
7377
ref="release/8.0",
7478
build_workflow="build.yml",
7579
publish_workflow="publish.yml",
@@ -88,6 +92,7 @@ def test_from_config_with_workflow_inputs(self) -> None:
8892
packages={
8993
"test-package": PackageConfig(
9094
repo="test/repo",
95+
package_type=PackageType.DEBIAN,
9196
build_workflow="build.yml",
9297
build_inputs={"key1": "value1", "key2": "value2"},
9398
publish_workflow="publish.yml",
@@ -113,6 +118,7 @@ def test_from_config_with_all_optional_fields(self) -> None:
113118
packages={
114119
"test-package": PackageConfig(
115120
repo="test/repo",
121+
package_type=PackageType.DEBIAN,
116122
ref="main",
117123
build_workflow="build.yml",
118124
build_timeout_minutes=60,
@@ -140,6 +146,7 @@ def test_from_config_with_empty_build_workflow(self) -> None:
140146
packages={
141147
"test-package": PackageConfig(
142148
repo="test/repo",
149+
package_type=PackageType.DEBIAN,
143150
build_workflow="",
144151
publish_workflow="publish.yml",
145152
)
@@ -156,6 +163,7 @@ def test_from_config_with_empty_publish_workflow(self) -> None:
156163
packages={
157164
"test-package": PackageConfig(
158165
repo="test/repo",
166+
package_type=PackageType.DEBIAN,
159167
build_workflow="build.yml",
160168
publish_workflow="",
161169
)
@@ -172,6 +180,7 @@ def test_from_config_with_whitespace_only_build_workflow(self) -> None:
172180
packages={
173181
"test-package": PackageConfig(
174182
repo="test/repo",
183+
package_type=PackageType.DEBIAN,
175184
build_workflow=" ",
176185
publish_workflow="publish.yml",
177186
)
@@ -188,6 +197,7 @@ def test_from_config_with_whitespace_only_publish_workflow(self) -> None:
188197
packages={
189198
"test-package": PackageConfig(
190199
repo="test/repo",
200+
package_type=PackageType.DEBIAN,
191201
build_workflow="build.yml",
192202
publish_workflow=" ",
193203
)
@@ -204,11 +214,13 @@ def test_from_config_with_multiple_packages(self) -> None:
204214
packages={
205215
"package1": PackageConfig(
206216
repo="test/repo1",
217+
package_type=PackageType.DEBIAN,
207218
build_workflow="build1.yml",
208219
publish_workflow="publish1.yml",
209220
),
210221
"package2": PackageConfig(
211222
repo="test/repo2",
223+
package_type=PackageType.DOCKER,
212224
build_workflow="build2.yml",
213225
publish_workflow="publish2.yml",
214226
),
@@ -230,6 +242,7 @@ def test_from_config_error_message_includes_package_name(self) -> None:
230242
packages={
231243
"my-special-package": PackageConfig(
232244
repo="test/repo",
245+
package_type=PackageType.DEBIAN,
233246
build_workflow="",
234247
publish_workflow="publish.yml",
235248
)
@@ -246,6 +259,7 @@ def test_from_config_with_boolean_build_workflow(self) -> None:
246259
packages={
247260
"test-package": PackageConfig(
248261
repo="test/repo",
262+
package_type=PackageType.DEBIAN,
249263
build_workflow=False,
250264
publish_workflow="publish.yml",
251265
)
@@ -262,6 +276,7 @@ def test_from_config_with_boolean_publish_workflow(self) -> None:
262276
packages={
263277
"test-package": PackageConfig(
264278
repo="test/repo",
279+
package_type=PackageType.DEBIAN,
265280
build_workflow="build.yml",
266281
publish_workflow=False,
267282
)
@@ -343,6 +358,7 @@ def test_release_state_ephemeral_not_serialized(self) -> None:
343358
packages={
344359
"test-package": PackageConfig(
345360
repo="test/repo",
361+
package_type=PackageType.DEBIAN,
346362
build_workflow="build.yml",
347363
publish_workflow="publish.yml",
348364
)
@@ -409,6 +425,7 @@ def test_ephemeral_field_exists(self) -> None:
409425
packages={
410426
"test-package": PackageConfig(
411427
repo="test/repo",
428+
package_type=PackageType.DEBIAN,
412429
build_workflow="build.yml",
413430
publish_workflow="publish.yml",
414431
)
@@ -425,6 +442,7 @@ def test_force_rebuild_field_can_be_modified(self) -> None:
425442
packages={
426443
"test-package": PackageConfig(
427444
repo="test/repo",
445+
package_type=PackageType.DEBIAN,
428446
build_workflow="build.yml",
429447
publish_workflow="publish.yml",
430448
)
@@ -442,6 +460,7 @@ def test_ephemeral_not_serialized(self) -> None:
442460
packages={
443461
"test-package": PackageConfig(
444462
repo="test/repo",
463+
package_type=PackageType.DEBIAN,
445464
build_workflow="build.yml",
446465
publish_workflow="publish.yml",
447466
)
@@ -468,6 +487,7 @@ def test_state_syncer_sets_tag_from_args(self) -> None:
468487
packages={
469488
"test-package": PackageConfig(
470489
repo="test/repo",
490+
package_type=PackageType.DEBIAN,
471491
build_workflow="build.yml",
472492
publish_workflow="publish.yml",
473493
)
@@ -487,11 +507,13 @@ def test_state_syncer_sets_force_rebuild_from_args(self) -> None:
487507
packages={
488508
"docker": PackageConfig(
489509
repo="test/docker",
510+
package_type=PackageType.DOCKER,
490511
build_workflow="build.yml",
491512
publish_workflow="publish.yml",
492513
),
493514
"redis": PackageConfig(
494515
repo="test/redis",
516+
package_type=PackageType.DEBIAN,
495517
build_workflow="build.yml",
496518
publish_workflow="publish.yml",
497519
),
@@ -512,16 +534,19 @@ def test_state_syncer_sets_multiple_force_rebuild_from_args(self) -> None:
512534
packages={
513535
"docker": PackageConfig(
514536
repo="test/docker",
537+
package_type=PackageType.DOCKER,
515538
build_workflow="build.yml",
516539
publish_workflow="publish.yml",
517540
),
518541
"redis": PackageConfig(
519542
repo="test/redis",
543+
package_type=PackageType.DEBIAN,
520544
build_workflow="build.yml",
521545
publish_workflow="publish.yml",
522546
),
523547
"snap": PackageConfig(
524548
repo="test/snap",
549+
package_type=PackageType.DEBIAN,
525550
build_workflow="build.yml",
526551
publish_workflow="publish.yml",
527552
),
@@ -543,6 +568,7 @@ def test_state_syncer_without_args(self) -> None:
543568
packages={
544569
"test-package": PackageConfig(
545570
repo="test/repo",
571+
package_type=PackageType.DEBIAN,
546572
build_workflow="build.yml",
547573
publish_workflow="publish.yml",
548574
)
@@ -565,16 +591,19 @@ def test_state_syncer_force_rebuild_all(self) -> None:
565591
packages={
566592
"docker": PackageConfig(
567593
repo="test/docker",
594+
package_type=PackageType.DOCKER,
568595
build_workflow="build.yml",
569596
publish_workflow="publish.yml",
570597
),
571598
"redis": PackageConfig(
572599
repo="test/redis",
600+
package_type=PackageType.DEBIAN,
573601
build_workflow="build.yml",
574602
publish_workflow="publish.yml",
575603
),
576604
"snap": PackageConfig(
577605
repo="test/snap",
606+
package_type=PackageType.DEBIAN,
578607
build_workflow="build.yml",
579608
publish_workflow="publish.yml",
580609
),
@@ -597,11 +626,13 @@ def test_state_syncer_force_rebuild_all_with_other_values(self) -> None:
597626
packages={
598627
"docker": PackageConfig(
599628
repo="test/docker",
629+
package_type=PackageType.DOCKER,
600630
build_workflow="build.yml",
601631
publish_workflow="publish.yml",
602632
),
603633
"redis": PackageConfig(
604634
repo="test/redis",
635+
package_type=PackageType.DEBIAN,
605636
build_workflow="build.yml",
606637
publish_workflow="publish.yml",
607638
),

0 commit comments

Comments
 (0)