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

feat: Add Timeseries Collection Support #2797

Merged
merged 3 commits into from
Aug 25, 2024

Conversation

massooti
Copy link

@massooti massooti commented Feb 20, 2024

This pull request introduces support for timeseries collections in MongoEngine, addressing issue #2661. With the addition of a 'timeseries' option in the Document meta dictionary, users can now easily create and manage timeseries collections. This enhancement aligns with MongoDB 5.0's timeseries feature, providing better support for time-based data storage scenarios. Contributions welcome!

Closes #2661 (if applicable)

@alm0ra
Copy link

alm0ra commented Aug 11, 2024

OMG, @massooti , could you please finalize this PR? :)

mongoengine/document.py Outdated Show resolved Hide resolved
mongoengine/document.py Outdated Show resolved Hide resolved
@@ -233,6 +235,28 @@ def _get_collection(cls):

return cls._collection

@classmethod
def _get_timeseries_collection(cls):
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add some tests, similarly to capped collection

Copy link
Author

Choose a reason for hiding this comment

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

Hi there,
I, added unit test for time-series collection and wanted to provide a quick update regarding the recent GitHub Actions failure. The issue appears to be related to timing rather than the test implementation itself. While the tests pass successfully in local environment, they occasionally fail in the CI environment due to timing constraints.

Please let me know if there are any adjustments or further steps you’d suggest. I’m happy to help troubleshoot or refine the implementation as needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this error in the CI
E pymongo.errors.OperationFailure: The field 'timeseries' is not a valid collection option. Options: { timeseries: { timeField: "timestamp", granularity: "seconds" }, expireAfterSeconds: 5 }, full error: {'ok': 0.0, 'errmsg': 'The field \'timeseries\' is not a valid collection option. Options: { timeseries: { timeField: "timestamp", granularity: "seconds" }, expireAfterSeconds: 5 }', 'code': 72, 'codeName': 'InvalidOptions'}

My guess is that the timeseries is not compatible with older version of MongoDB so you need to decorate the tests with e.g requires_mongodb_gte_44

Copy link
Author

@massooti massooti Aug 25, 2024

Choose a reason for hiding this comment

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

Thanks for your feedback, i would use this decorators, but actually mogodb starts to supports timeseries collection from version 5.0, MongoDB 5.0 in the official MongoDB documentation, So To ensure the codebase remains scalable and future-proof, I think it would be beneficial to introduce a new decorator similar to the existing ones for MongoDB version checks.

what do you think, are you agree for adding new decorators for specifically timeseries collection supports like this requires_mongodb_gte_50,
and for other upper versions which already exsits
requires_mongodb_gte_60,
requires_mongodb_gte_70.

Please let me know your thoughts on this suggestion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure , feel free to add as needed

Copy link
Author

Choose a reason for hiding this comment

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

Dear @bagerard
For now all is Done and its ready to merge

@bagerard bagerard merged commit 64c28ef into MongoEngine:master Aug 25, 2024
20 checks passed
@bagerard
Copy link
Collaborator

bagerard commented Aug 25, 2024

Looks good to me , thanks for the contribution 👍

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.

Timeseries collection
3 participants