-
Notifications
You must be signed in to change notification settings - Fork 1
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
OpenSearch indexes partitioned to daily partitions #4
Conversation
xdaniel3
commented
Oct 31, 2023
- allow to match them through pattern matching
3ff7082
to
a7e9f1c
Compare
Pull Request Test Coverage Report for Build 6796045226
💛 - Coveralls |
c3caaa9
to
47959df
Compare
47959df
to
350dab1
Compare
- allow to match them through pattern matching
350dab1
to
071ed96
Compare
@@ -3,6 +3,7 @@ | |||
from django.test.utils import override_settings | |||
|
|||
from .models import CommandLog, CeleryTaskRunLog, CeleryTaskInvocationLog, InputRequestLog, OutputRequestLog | |||
from .models import PartitionedLog |
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.
why is there another import line instead of an addition to the one above?
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.
line length would be too long
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.
why not use parentheses instead? they are used elsewhere int he project
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.
Ok, but let's incorporate some linting, it's hard to search in project what conventions should be used.
template = document_class.get_template() | ||
template.save() | ||
else: | ||
document_class.init() |
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.
Wouldn't it be better to override the init
method of the PartitionedLog
and save the template there? You could remove this whole if/else change in both places and it would comform to the pattern of the other Documents.
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.
init
is particularly for creating index, while I create here just a template, so it might be confusing imho. This implementation is directly from documentation of elasticsearch-dsl
for time-based indexes.
But it could be in some function, that's for sure.