Change load and save methods on MongoDBPersister to show partition_key argument as optional

[kicksent]

This change was made because the default value for partition is None but the mongo persister did not have None in the type annotation. This change brings the function signature in line with the other persistors in the project that allow None as a valid value for the partition key.

Changes

Changed partition key to optional.

How I tested this

pytest -v tests/integrations/persisters/test_b_mongodb.py

Notes

The partition_key default is None, so this will help to make it clear.

Glad to be able to contribute! :)

Checklist

• PR has an informative and human-readable title (this will be pulled into the release notes)
• Changes are limited to a single goal (no scope creep)
• Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
• Any change in functionality is tested
• [none] New functions are documented (with a description, list of inputs, and expected output)
• [none] Placeholder code is flagged / future TODOs are captured in comments
• [none] Project documentation has been updated if adding/changing functionality.

---

Important

Make partition_key optional in load and save methods of MongoDBBasePersister, with tests for None value.

• Behavior:
  • partition_key argument in load() and save() methods of MongoDBBasePersister is now optional.
  • Default value for partition_key is None.
• Tests:
  • Added test_partition_key_is_optional() in test_b_mongodb.py to verify load() and save() with partition_key=None.

^{This description was created by for 78f5d4f. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 78f5d4f in 1 minute and 13 seconds

More details

• Looked at 43 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. burr/integrations/persisters/b_pymongo.py:99

• Draft comment:
  Docstring for the load() method should mention that partition_key can be None.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. burr/integrations/persisters/b_pymongo.py:121

• Draft comment:
  Update the save() method docstring to indicate that partition_key is optional.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

3. tests/integrations/persisters/test_b_mongodb.py:70

• Draft comment:
  Good test addition for verifying partition_key can be None; consider adding explanatory comments if needed.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

4. burr/integrations/persisters/b_pymongo.py:120

• Draft comment:
  Similarly, update the docstring in the save method to indicate that partition_key is optional and clarify the behavior when None is provided.
• Reason this comment was not posted:
  Marked as duplicate.

5. tests/integrations/persisters/test_b_mongodb.py:72

• Draft comment:
  The test uses 'in_progress' as a status value, but the save method's type hint accepts only 'completed' or 'failed'. Please update the test or adjust the allowed statuses accordingly.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:

1. The comment refers to type hints that aren't visible in the provided code. 2. The test file shows multiple status values being used successfully. 3. Without seeing the actual implementation and type hints of the save method, we can't verify this claim. 4. The fact that tests pass with different status values suggests the comment might be incorrect.
   I haven't seen the actual implementation file with the type hints, so I can't be 100% certain the comment is wrong.
   However, the fact that multiple tests in this file use different status values successfully, and that this is a test file that presumably would fail if the type hints were violated, strongly suggests the comment is incorrect.
   Delete the comment because we don't have strong evidence that it's correct, and the existing tests suggest it's likely wrong.

Workflow ID: wflow_iNAxMi0r1jRtrXc4

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[skrawcz]

@kicksent thanks. One request, could you update this PR with a message I could use for the squash commit just to elaborate a little on why we're doing this change please? If you don't get to it, I'll do it. Otherwise we'll get this out next week. I think the docs failure is just a permissions thing.

[kicksent]

This change was made because the default value for partition is None but the mongo persister did not have None in the type annotation. This change brings the function signature in line with the other persistors in the project that allow None as a valid value for the partition key.

This change was made because the default value for partition is None but the mongo persister did not have None in the type annotation. This change brings the function signature in line with the other persistors in the project that allow None as a valid value for the partition key.