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.

Sign up for GitHub

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

Jump to bottom

Bulk Writer improving & bulk_writer method for document and possibility to bypass mongo document validation + comment parameter #1079

Merged
merged 26 commits into from
Dec 25, 2024
Merged

Bulk Writer improving & bulk_writer method for document and possibility to bypass mongo document validation + comment parameter #1079

merged 26 commits into from
Dec 25, 2024
+292 −110

Conversation

CAPITAINMARVEL
Copy link
Contributor

@CAPITAINMARVEL CAPITAINMARVEL commented Dec 6, 2024 •
edited
Loading

  • Reworked it
    added:
  • bypass_document_validation
  • comment parameter
  • method to use bulk_write on Document and UnionDoc model

handle
#327

@CAPITAINMARVEL CAPITAINMARVEL marked this pull request as ready for review December 8, 2024 00:15
@CAPITAINMARVEL
Copy link
Contributor Author

added #327

staticxterm
staticxterm requested changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@staticxterm staticxterm left a comment

Choose a reason for hiding this comment

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

The BulkWriter changes look good, needs some more polishing...

beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
tests/odm/documents/test_bulk_write.py Outdated Show resolved Hide resolved
tests/odm/documents/test_bulk_write.py Outdated Show resolved Hide resolved
tests/odm/documents/test_bulk_write.py Outdated Show resolved Hide resolved
tests/odm/documents/test_bulk_write.py Outdated Show resolved Hide resolved
staticxterm
staticxterm requested changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@staticxterm staticxterm left a comment

Choose a reason for hiding this comment

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

Looks good, have one final nitpick about one test assertion and some typing improvements if you agree...

tests/odm/documents/test_bulk_write.py Outdated Show resolved Hide resolved
beanie/odm/union_doc.py Outdated Show resolved Hide resolved
beanie/odm/documents.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
staticxterm
staticxterm previously approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@staticxterm staticxterm left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@CAPITAINMARVEL CAPITAINMARVEL enabled auto-merge (squash) December 13, 2024 00:23
@CAPITAINMARVEL CAPITAINMARVEL enabled auto-merge (squash) December 13, 2024 00:26
adeelsohailahmed
adeelsohailahmed previously approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@adeelsohailahmed adeelsohailahmed left a comment

Choose a reason for hiding this comment

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

Good stuff! Please update the PR's title to be a bit more descriptive for the change log.

@adeelsohailahmed adeelsohailahmed requested a review from a team December 13, 2024 01:30
@CAPITAINMARVEL CAPITAINMARVEL changed the title Bulk Writer Improving Beanie Bulk Writer improving, bulk_writer method for document and possibility to use parameter bypass_document_validation and comment Dec 13, 2024
@CAPITAINMARVEL CAPITAINMARVEL changed the title Beanie Bulk Writer improving, bulk_writer method for document and possibility to use parameter bypass_document_validation and comment Bulk Writer improving & bulk_writer method for document Dec 13, 2024
@CAPITAINMARVEL CAPITAINMARVEL changed the title Bulk Writer improving & bulk_writer method for document Bulk Writer improving & bulk_writer method for document and possibility to bypass mongo document validation + comment parameter Dec 13, 2024
roman-right
roman-right requested changes Dec 22, 2024
View reviewed changes
Copy link
Member

@roman-right roman-right left a comment

Choose a reason for hiding this comment

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

Please take a look at the few comments I left.
In general, it is a great job! Thank you!

beanie/odm/documents.py Show resolved Hide resolved
beanie/odm/union_doc.py Show resolved Hide resolved
beanie/odm/bulk.py Outdated Show resolved Hide resolved
@adeelsohailahmed adeelsohailahmed dismissed their stale review December 23, 2024 19:17

Implement Roman's requested changes

@CAPITAINMARVEL CAPITAINMARVEL merged commit 4bf5d56 into main Dec 25, 2024
61 checks passed
@CAPITAINMARVEL CAPITAINMARVEL deleted the bulk_write_rework branch December 25, 2024 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-right roman-right roman-right approved these changes

@adeelsohailahmed adeelsohailahmed adeelsohailahmed approved these changes

@staticxterm staticxterm staticxterm approved these changes

@07pepa 07pepa Awaiting requested review from 07pepa

@Riverfount Riverfount Awaiting requested review from Riverfount

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CAPITAINMARVEL @staticxterm @adeelsohailahmed @roman-right