-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
refactor: doc content hash #1616
Conversation
This PR closes: #1615 |
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
Codecov Report
@@ Coverage Diff @@
## master #1616 +/- ##
==========================================
+ Coverage 84.11% 85.02% +0.91%
==========================================
Files 127 127
Lines 6654 6699 +45
==========================================
+ Hits 5597 5696 +99
+ Misses 1057 1003 -54
Continue to review full report at Codecov.
|
def update_content_hash(self): | ||
"""Update the document hash according to its content.""" | ||
self._document.content_hash = get_content_hash(self._document) | ||
def update_content_hash(self, mask: Tuple[str] = ('id', 'chunks', 'matches', 'content_hash')) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened the issue to have exactly the opposite logic, I want the mask to be the fields that we want to include and not the ones we want to exclude.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened the issue to have exactly the opposite logic, I want the mask to be the fields that we want to include and not the ones we want to exclude.
I also think this is more resilient to any future adding of new fields
refactor on #1611