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

support dict merging in fieldmanager and add_fields_to method #737

Merged
merged 10 commits into from
Jan 3, 2025

Conversation

dtrai2
Copy link
Collaborator

@dtrai2 dtrai2 commented Dec 23, 2024

  • allow the FieldManager to merge dicts
  • rename extend_target_list to merge_with_target (breaking change)

- Simplified and streamlined logic for merging fields and resolving overwrite/extend conflicts.
- Improved support for merging dictionaries and extending lists in `add_fields_to`.
- Added tests to validate new behaviors and handle edge cases more effectively.
- Replaced `extend_target_list` with `merge_with_target` across processors and utilities for clarity and better functionality.
- Ensured proper handling of scenarios where merging and overwriting targets could conflict.
- Ensures all values in the `prefixed_label` dictionary are lists.
- Adds a unit test to validate the behavior for labeler rules.
@dtrai2 dtrai2 self-assigned this Dec 23, 2024
- moves test at the end of the list
- Simplifies conditional branches by reordering merge cases.
- Streamlines how source values and targets are handled.
- Ensures proper handling of dict merging and overwrite flags.
@dtrai2 dtrai2 linked an issue Dec 23, 2024 that may be closed by this pull request
- Introduced a new test case to validate merging response data with an existing dictionary.
- Ensures proper behavior when "merge_with_target" is enabled in the requester configuration.
@dtrai2 dtrai2 changed the title support dict merging in fieldmanager support dict merging in fieldmanager and add_fields_to method Dec 23, 2024
@dtrai2 dtrai2 requested a review from ekneg54 December 23, 2024 17:22
@dtrai2 dtrai2 marked this pull request as ready for review December 23, 2024 17:22
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (a4eb13d) to head (ae7546f).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
logprep/util/helper.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #737      +/-   ##
==========================================
+ Coverage   94.32%   96.57%   +2.25%     
==========================================
  Files         146      145       -1     
  Lines        9724     9340     -384     
==========================================
- Hits         9172     9020     -152     
+ Misses        552      320     -232     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ekneg54 ekneg54 merged commit 33f6cb2 into main Jan 3, 2025
13 checks passed
@ekneg54 ekneg54 deleted the 649-fieldmanager-union-overwrite-dict branch January 3, 2025 09:33
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.

FieldManager: Union overwrite dict
3 participants