-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Trigger Geopoint ES Index on Geospatial Feature Flag Enable #35126
Open
zandre-eng
wants to merge
29
commits into
master
Choose a base branch
from
ze/trigger-es-index-geospatial-enable
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
4b32aec
extend task tracker to handle messages
zandre-eng 6280c17
refactor index command functions
zandre-eng 7f52f0e
helper func to get celery task tracker
zandre-eng 2184e76
start celery task to index docs on enabling feature flag
zandre-eng d7abcee
send task message to front-end
zandre-eng 5409b02
display task message
zandre-eng 177b1b3
unit tests
zandre-eng 9deab96
setting for max index doc limit
zandre-eng b647463
error status for task helper
zandre-eng 196ffd3
simplify tracker keys
zandre-eng 6906d82
use progress instead of messages
zandre-eng ca4ad45
update unit tests
zandre-eng 128757f
fix incorrect unit test assert
zandre-eng edb3a61
use local references
zandre-eng a4c7bb5
remove unused reference
zandre-eng 707a97d
increase timeouts
zandre-eng ded63ee
keep track of error slug to show different messages
zandre-eng a161b10
move function
zandre-eng a030555
Merge remote-tracking branch 'origin/ze/trigger-es-index-geospatial-e…
ajeety4 29ec236
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng 037e5f2
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng 0724d43
fix import
zandre-eng d96c1d4
add notify_exception for processing failure
zandre-eng e1d82cc
make progress output optional
zandre-eng 5137009
use geospatial queue
zandre-eng dd28ee8
account for batches that are smaller than query limit
zandre-eng 1581ca5
add offset for query
zandre-eng cbe08cf
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng e1ef27a
remove refreshing index
zandre-eng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 the number should be fine, but we should atleast have some benchmarks on the number, if it affects the ES cluster in anyway.
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.
Sounds good. I'll see if QA is able to do some load testing, otherwise I'll look into doing this.
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.
No need to change this here, but just useful to know about. Since Python 3.6, you can use underscores to separate thousands in decimals (and words in hexadecimals and nibbles in binaries) to make numbers more readable. e.g.
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.
@kaapstorm Initially this is how I had it but then ultimately decided against it since I didn't see this formatting being used anywhere else within the
settings.py
file. I think this is fine to keep as is, but good to keep in mind for next time!