Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make time out in delete assets to be configurable #82

Merged
merged 12 commits into from
Sep 2, 2024

Conversation

luthfifahlevi
Copy link

No description provided.

cli/config.go Outdated Show resolved Hide resolved
@luthfifahlevi luthfifahlevi force-pushed the feat-configurable-time-out-in-delete-assets branch from 81f5ad7 to 9ee9390 Compare September 2, 2024 08:49
@coveralls
Copy link

coveralls commented Sep 2, 2024

Pull Request Test Coverage Report for Build 10666882632

Details

  • 3 of 11 (27.27%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 84.02%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core/asset/service.go 3 6 50.0%
core/asset/config.go 0 5 0.0%
Totals Coverage Status
Change from base Build 10504081306: -0.05%
Covered Lines: 6877
Relevant Lines: 8185

💛 - Coveralls

Copy link

@irainia irainia left a comment

Choose a reason for hiding this comment

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

Rest looks good

cli/config.go Outdated Show resolved Hide resolved
@luthfifahlevi luthfifahlevi force-pushed the feat-configurable-time-out-in-delete-assets branch from dbad4f8 to 0f0b609 Compare September 2, 2024 10:05
cli/server.go Outdated
LineageRepo: lineageRepository,
Worker: wrkr,
Logger: logger,
DeleteAssetsTimeout: cfg.Asset.DeleteAssetsTimeout,
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's pass the cfg altogether

@@ -19,6 +19,7 @@ type Service struct {
lineageRepository LineageRepository
worker Worker
logger log.Logger
assetConfig Config
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's avoid unnecessary context, we can use config as name

Suggested change
assetConfig Config
config Config

@luthfifahlevi luthfifahlevi force-pushed the feat-configurable-time-out-in-delete-assets branch from d4b7f54 to fbc4a5e Compare September 2, 2024 12:11
@luthfifahlevi luthfifahlevi merged commit be09caf into main Sep 2, 2024
3 checks passed
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.

4 participants