Skip to content

[unstable] Add File size Config#476

Merged
Hmnt39 merged 15 commits intomasterfrom
DOG-5997-replicate-config
Sep 24, 2025
Merged

[unstable] Add File size Config#476
Hmnt39 merged 15 commits intomasterfrom
DOG-5997-replicate-config

Conversation

@Hmnt39
Copy link
Copy Markdown
Contributor

@Hmnt39 Hmnt39 commented Sep 10, 2025

This PR moves the FileSizeConfig to the unstable configuration.
Remaining config are already moved to unstable with Statestore and Metrics PR.

@Hmnt39 Hmnt39 requested a review from a team as a code owner September 10, 2025 07:25
@einarmo
Copy link
Copy Markdown
Contributor

einarmo commented Sep 10, 2025

Doesn't the new unstable config use pydantic? I don't think this will work.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 10, 2025

Codecov Report

❌ Patch coverage is 75.71429% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.59%. Comparing base (67ffc88) to head (c2314a7).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...te/extractorutils/unstable/configuration/models.py 75.71% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
- Coverage   80.67%   80.59%   -0.09%     
==========================================
  Files          43       43              
  Lines        4124     4194      +70     
==========================================
+ Hits         3327     3380      +53     
- Misses        797      814      +17     
Files with missing lines Coverage Δ
...te/extractorutils/unstable/configuration/models.py 83.72% <75.71%> (-2.05%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Hmnt39
Copy link
Copy Markdown
Contributor Author

Hmnt39 commented Sep 10, 2025

Doesn't the new unstable config use pydantic? I don't think this will work.

Make changes according to pydantic.

@Hmnt39 Hmnt39 requested a review from einarmo September 19, 2025 10:57
@Hmnt39 Hmnt39 requested a review from einarmo September 23, 2025 06:11
Copy link
Copy Markdown
Contributor

@einarmo einarmo left a comment

Choose a reason for hiding this comment

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

Could you add tests for using a pydantic field as well? I.e. that having a default works as expected. Also, could you write a test that verifies the equality logic? I.e. FileSizeConfig("2000MB") == FileSizeConfig("2GB")

@Hmnt39
Copy link
Copy Markdown
Contributor Author

Hmnt39 commented Sep 23, 2025

Could you add tests for using a pydantic field as well? I.e. that having a default works as expected. Also, could you write a test that verifies the equality logic? I.e. FileSizeConfig("2000MB") == FileSizeConfig("2GB")

Added more test cases to check the defaults and equality logic.

einarmo
einarmo previously approved these changes Sep 23, 2025
Copy link
Copy Markdown
Contributor

@einarmo einarmo left a comment

Choose a reason for hiding this comment

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

As a final note, you should mention in the PR description that this migrates an existing class to use pydantic, and link to the original config type.

@Hmnt39 Hmnt39 added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Sep 23, 2025
@frxstrem frxstrem self-assigned this Sep 24, 2025
@frxstrem frxstrem added the risk-review-ongoing Risk review is in progress label Sep 24, 2025
@frxstrem frxstrem removed the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Sep 24, 2025
@frxstrem frxstrem added the waiting-for-team Waiting for the submitter or reviewer of the PR to take an action label Sep 24, 2025
Copy link
Copy Markdown

@frxstrem frxstrem left a comment

Choose a reason for hiding this comment

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

🦄

@Hmnt39 Hmnt39 merged commit d01ca0a into master Sep 24, 2025
8 of 9 checks passed
@Hmnt39 Hmnt39 deleted the DOG-5997-replicate-config branch September 24, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants