Skip to content

Conversation

@benkeanna
Copy link
Contributor

No description provided.

@benkeanna benkeanna force-pushed the aben/aws-iam-role branch 6 times, most recently from fd9648a to 8f10053 Compare September 3, 2025 08:50
janmatzek
janmatzek previously approved these changes Sep 3, 2025
lt=3,
description="Max workers must be greater than 0 and less than 3",
),
] = Field(default=ConcurrencyDefaults.MAX_WORKERS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: You can use the BackupSettings.MAX_WORKERS, BackupSettings inherits the ConcurrencyDefaults class... although I admit that the inheritance detracts from readability here

description="Batch size must be greater than 0",
),
] = Field(default=BackupSettings.DEFAULT_BATCH_SIZE)
max_workers: Annotated[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the BackupManager class uses BackupSettings.MAX_WORKERS to limit the threads, not the value from BackupRestoreConfig... can you please change that in backup_manager.py?
Currently it has LN347: max_workers=BackupSettings.MAX_WORKERS which should be something like max_workers=config.max_workers

@benkeanna benkeanna merged commit 4f27fa5 into master Sep 3, 2025
9 checks passed
@benkeanna benkeanna deleted the aben/aws-iam-role branch September 3, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants