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(cts): add tests for chunkedBatch wrappers #3268

Merged
merged 10 commits into from
Jul 8, 2024
Merged

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Jun 28, 2024

🧭 What and Why

🎟 JIRA Ticket: DI-2506

Generate helpers for all the wrapper helpers and assert the requests with a mock server

@millotp millotp self-assigned this Jun 28, 2024
@millotp millotp requested a review from a team as a code owner June 28, 2024 09:41
@millotp millotp requested review from Fluf22 and shortcuts June 28, 2024 09:41
@algolia-bot
Copy link
Collaborator

algolia-bot commented Jun 28, 2024

🔨 The codegen job will run at the end of the CI.

Make sure your last commit does not contain generated code, it will be automatically pushed by our CI.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Jul 4, 2024

✔️ Code generated!

Name Link
🪓 Triggered by 85d5b7af5b63082f248340a022d4ae49536bf751
🍃 Generated commit 7f1dfc5f11302d7e9fba9f89aeb7cf8cb39b6d4f
🌲 Generated branch generated/feat/helper-test

Copy link

github-actions bot commented Jul 4, 2024

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

awesome

Comment on lines +66 to +71
copyCount: 2,
batchCount: 10,
waitTaskCount: 6,
tmpIndexName: req.params.indexName,
waitingForFinalWaitTask: false,
successful: false,
Copy link
Member

Choose a reason for hiding this comment

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

next up: JSON based generator for the test server

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coming soon ™️ !

@@ -1,5 +1,5 @@
method:
post:
get:
Copy link
Member

Choose a reason for hiding this comment

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

it's a post, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its neither, its a helper, there is no method associated with it, but for consistency I put get everywhere.
For now it doesn't really matter but if in the script we expect all helpers to be GET it makes like easier

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for example for replaceAllObjects that call multiple endpoints its not possible to tell what is the method, its only here to comply with redocly

Copy link
Member

Choose a reason for hiding this comment

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

yup it's just that it seems wrong when reading the specs, we should maybe just add a disclaimer somewhere that those specs are only for generating models (like in the folder root directory)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do in next PR !

@@ -265,7 +265,7 @@
"""
Helper: Replaces object content of all the given objects according to their respective `objectID` field. The `chunked_batch` helper is used under the hood, which creates a `batch` requests with at most 1000 objects in it.
"""
return await self.chunked_batch(index_name=index_name, objects=objects, action=Action.PARTIALUPDATEOBJECT and create_if_not_exists or Action.PARTIALUPDATEOBJECTNOCREATE)
return await self.chunked_batch(index_name=index_name, objects=objects, action=Action.PARTIALUPDATEOBJECT if create_if_not_exists else Action.PARTIALUPDATEOBJECTNOCREATE)
Copy link
Member

Choose a reason for hiding this comment

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

possessed by lua

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

all good for me then, gg for the fixes along the way!

@millotp millotp merged commit 07266f2 into main Jul 8, 2024
20 checks passed
@millotp millotp deleted the feat/helper-test branch July 8, 2024 09:42
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.

3 participants