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

Trigger Geopoint ES Index on Geospatial Feature Flag Enable #35126

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
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 Sep 12, 2024
6280c17
refactor index command functions
zandre-eng Sep 12, 2024
7f52f0e
helper func to get celery task tracker
zandre-eng Sep 12, 2024
2184e76
start celery task to index docs on enabling feature flag
zandre-eng Sep 12, 2024
d7abcee
send task message to front-end
zandre-eng Sep 12, 2024
5409b02
display task message
zandre-eng Sep 12, 2024
177b1b3
unit tests
zandre-eng Sep 13, 2024
9deab96
setting for max index doc limit
zandre-eng Sep 16, 2024
b647463
error status for task helper
zandre-eng Sep 16, 2024
196ffd3
simplify tracker keys
zandre-eng Sep 17, 2024
6906d82
use progress instead of messages
zandre-eng Sep 17, 2024
ca4ad45
update unit tests
zandre-eng Sep 17, 2024
128757f
fix incorrect unit test assert
zandre-eng Sep 17, 2024
edb3a61
use local references
zandre-eng Sep 20, 2024
a4c7bb5
remove unused reference
zandre-eng Sep 20, 2024
707a97d
increase timeouts
zandre-eng Sep 20, 2024
ded63ee
keep track of error slug to show different messages
zandre-eng Sep 23, 2024
a161b10
move function
zandre-eng Oct 1, 2024
a030555
Merge remote-tracking branch 'origin/ze/trigger-es-index-geospatial-e…
ajeety4 Oct 9, 2024
29ec236
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng Oct 17, 2024
037e5f2
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng Oct 21, 2024
0724d43
fix import
zandre-eng Oct 21, 2024
d96c1d4
add notify_exception for processing failure
zandre-eng Oct 24, 2024
e1d82cc
make progress output optional
zandre-eng Oct 30, 2024
5137009
use geospatial queue
zandre-eng Nov 6, 2024
dd28ee8
account for batches that are smaller than query limit
zandre-eng Nov 6, 2024
1581ca5
add offset for query
zandre-eng Nov 6, 2024
cbe08cf
Merge branch 'master' into ze/trigger-es-index-geospatial-enable
zandre-eng Nov 6, 2024
e1ef27a
remove refreshing index
zandre-eng Nov 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2093,3 +2093,5 @@ def _pkce_required(client_id):

# NOTE: if you are adding a new setting that you intend to have other environments override,
# make sure you add it before localsettings are imported (from localsettings import *)

MAX_GEOSPATIAL_INDEX_DOC_LIMIT = 1000000
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

MAX_GEOSPATIAL_INDEX_DOC_LIMIT = 1_000_000
addr = 0xCAFE_F00D
flags = 0b_0011_1111_0100_1110

Copy link
Contributor Author

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!