fix: handle BlobDelete event type in `batch_push_results` #893

liammoat · 2024-05-13T16:12:52Z

Purpose

batch_push_results receives, but does not handle, blob delete events.
This PR introduces the functionality to delete the search index, when a blob is deleted from Azure Storage

Does this introduce a breaking change?

Yes
No

How to Test

Run the updated Azure Function
#1 Create a blob, uploading via Azure Storage
#2 Delete the blob from Azure Storage
#3 Trigger `BatchStartProcessing function

What to Check

Verify that the following are valid

#1 BatchPushResults is triggered, and the document is indexed.
#2 BatchPushResults is triggered, and the associated indexes are removed.
#3 BatchPushResults is triggered, as normal, and the documents are indexed.

Other Information

Remaining Actions

The following is outstanding for this PR:

Tests

github-actions · 2024-05-13T16:14:46Z

Coverage Report •

File	Stmts	Miss	Cover	Missing
code/backend/batch
batch_push_results.py	36	0	100%
function_app.py	17	17	0%	1–8, 10, 12–14, 16, 19–22
code/backend/batch/utilities/search
search_handler_base.py	48	23	52%	13–15, 18–20, 24, 28, 32, 36, 40, 44, 48, 51–52, 54–56, 58–59, 62–63, 65
TOTAL	2424	684	71%

Tests	Skipped	Failures	Errors	Time
201	0 💤	0 ❌	0 🔥	12.926s ⏱️

liammoat · 2024-05-13T16:21:42Z

Team - Couple of things to note about this PR:

There is likely a better way to do this - however, I have tried to implement it with as few downstream changes as possible (i.e. I don't want to refactor everything to fix this)
With this change in mind, I propose the admin app should delete the blob, and let the function handle the index.
I will leave this in draft until I have introduce some tests to cover the core behaviour

code/backend/batch/BatchPushResults.py

gaurarpit

nice, looks good. some comments.

code/backend/batch/function_app.py

superhindupur · 2024-05-14T09:25:42Z

Team - Couple of things to note about this PR:

There is likely a better way to do this - however, I have tried to implement it with as few downstream changes as possible (i.e. I don't want to refactor everything to fix this)

With this change in mind, I propose the admin app should delete the blob, and let the function handle the index.

I will leave this in draft until I have introduce some tests to cover the core behaviour

Definitely agree with " I propose the admin app should delete the blob, and let the function handle the index." - could you please create an issue to cover this if there isn't one already?

Update: I see that @komalg1 is working on this in #876 - would be worth the two of you catching up.

superhindupur · 2024-05-14T09:26:30Z

@liammoat thanks so much for this, really appreciate it! ❤️

adamdougal

code/backend/batch/utilities/search/SearchHandlerBase.py

code/tests/test_BatchPushResults.py

code/backend/batch/BatchPushResults.py

Co-authored-by: Arpit Gaur <gaurarpit@gmail.com>

…les PEP8 compliant (#899)

Co-authored-by: Adam Dougal <adamdougal@microsoft.com> Co-authored-by: Adam Dougal <adamdougal@users.noreply.github.com> Co-authored-by: Arpit Gaur <arpitgaur@microsoft.com>

…k it as is. (#912)

…922)

Co-authored-by: Arpit Gaur <arpitgaur@microsoft.com>

Co-authored-by: Arpit Gaur <gaurarpit@gmail.com>

…bdeleted-events

.vscode/settings.json

cecheta

Great work

feat: handle BlobDelete event type in batch_push_results

a62bbb6

liammoat linked an issue May 13, 2024 that may be closed by this pull request

Handle BlobDeleted events from Event Grid #414

Closed

liammoat added 3 commits May 13, 2024 19:19

tests: initial tests for batch_push_results control logic

4740dac

Merge branch 'main' into limoat/414/handle-blobdeleted-events

590c2d7

tests: additional tests for batch push results

5c28dc7

liammoat changed the title ~~[DRAFT] fix: handle BlobDelete event type in batch_push_results~~ fix: handle BlobDelete event type in batch_push_results May 14, 2024

liammoat marked this pull request as ready for review May 14, 2024 07:45

liammoat self-assigned this May 14, 2024

gaurarpit reviewed May 14, 2024

View reviewed changes

code/backend/batch/BatchPushResults.py Outdated Show resolved Hide resolved

gaurarpit reviewed May 14, 2024

View reviewed changes

code/backend/batch/BatchPushResults.py Outdated Show resolved Hide resolved

gaurarpit reviewed May 14, 2024

View reviewed changes

liammoat added 2 commits May 14, 2024 09:17

refactor: remove redundant error log

1afac88

fix: update logger to debug

5b4b15f

superhindupur reviewed May 14, 2024

View reviewed changes

code/backend/batch/function_app.py Show resolved Hide resolved

superhindupur mentioned this pull request May 14, 2024

delete from storage #876

Closed

2 tasks

adamdougal previously approved these changes May 14, 2024

View reviewed changes

Merge branch 'main' into limoat/414/handle-blobdeleted-events

3281ef0

gaurarpit previously approved these changes May 14, 2024

View reviewed changes

cecheta reviewed May 15, 2024

View reviewed changes

code/backend/batch/utilities/search/SearchHandlerBase.py Outdated Show resolved Hide resolved

code/tests/test_BatchPushResults.py Outdated Show resolved Hide resolved

cecheta reviewed May 16, 2024

View reviewed changes

code/backend/batch/BatchPushResults.py Outdated Show resolved Hide resolved

liammoat and others added 6 commits May 16, 2024 10:20

refactor: remove redundant line in delete by source

63026d8

Co-authored-by: Arpit Gaur <gaurarpit@gmail.com>

refactor: Make backend search, backend tools and remainder backend fi…

a220d39

…les PEP8 compliant (#899)

feat: Generate embeddings for images (#892)

1315799

Co-authored-by: Adam Dougal <adamdougal@microsoft.com> Co-authored-by: Adam Dougal <adamdougal@users.noreply.github.com> Co-authored-by: Arpit Gaur <arpitgaur@microsoft.com>

ci: Add stale bot (#918)

9c8d5aa

fix: Keep the Admin.py as uppercase naming to allow the streamlit pic…

8762f24

…k it as is. (#912)

fix: Fix computer vision for deployments (#919)

31bc18b

gaurarpit and others added 4 commits May 16, 2024 10:21

refactor: Apply pep8 on leftover files from pr-merge (#908)

676578d

fix: Fix generate_arm_templates.sh script, reformat all Bicep files (#…

e790daa

…922)

feat: Store image embeddings in search index (#921)

9940c91

Co-authored-by: Arpit Gaur <arpitgaur@microsoft.com>

test: update test_batch_push_results following merge

ada413d

Co-authored-by: Arpit Gaur <gaurarpit@gmail.com>

liammoat dismissed stale reviews from gaurarpit and adamdougal via ada413d May 16, 2024 10:22

liammoat added 3 commits May 16, 2024 11:29

Merge remote-tracking branch 'origin/main' into limoat/414/handle-blo…

8343518

…bdeleted-events

refactor: move import order

cf2589e

Merge remote-tracking branch 'origin/main' into limoat/414/handle-blo…

2c0db27

…bdeleted-events

liammoat requested review from adamdougal and superhindupur May 16, 2024 10:53

chore: add pylint from selected interpreter

1adbd0a

liammoat requested a review from ross-p-smith May 16, 2024 11:19

refactor: remove any

fae0701

ross-p-smith reviewed May 16, 2024

View reviewed changes

.vscode/settings.json Outdated Show resolved Hide resolved

chore: use in-project for poetry

3ceaeb9

cecheta previously approved these changes May 16, 2024

View reviewed changes

chore: remove in-project env

66b3bf0

liammoat dismissed cecheta’s stale review via 66b3bf0 May 16, 2024 12:49

liammoat requested review from ross-p-smith and cecheta May 16, 2024 13:07

cecheta approved these changes May 16, 2024

View reviewed changes

gaurarpit self-assigned this May 16, 2024

gaurarpit added this pull request to the merge queue May 16, 2024

Merged via the queue into main with commit f27b68e May 16, 2024
9 checks passed

gaurarpit deleted the limoat/414/handle-blobdeleted-events branch May 16, 2024 15:26

frtibble mentioned this pull request May 16, 2024

chore(main): release 1.2.0 #906

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: handle BlobDelete event type in `batch_push_results` #893

fix: handle BlobDelete event type in `batch_push_results` #893

liammoat commented May 13, 2024 •

edited

github-actions bot commented May 13, 2024 •

edited

liammoat commented May 13, 2024

gaurarpit left a comment

superhindupur commented May 14, 2024 •

edited

superhindupur commented May 14, 2024

adamdougal left a comment

cecheta left a comment

fix: handle BlobDelete event type in batch_push_results #893

fix: handle BlobDelete event type in batch_push_results #893

Conversation

liammoat commented May 13, 2024 • edited

Purpose

Does this introduce a breaking change?

How to Test

What to Check

Other Information

Remaining Actions

github-actions bot commented May 13, 2024 • edited

liammoat commented May 13, 2024

gaurarpit left a comment

Choose a reason for hiding this comment

superhindupur commented May 14, 2024 • edited

superhindupur commented May 14, 2024

adamdougal left a comment

Choose a reason for hiding this comment

cecheta left a comment

Choose a reason for hiding this comment

fix: handle BlobDelete event type in `batch_push_results` #893

fix: handle BlobDelete event type in `batch_push_results` #893

liammoat commented May 13, 2024 •

edited

github-actions bot commented May 13, 2024 •

edited

superhindupur commented May 14, 2024 •

edited