Skip to content

feat: add transformation bridge methods #1586

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

Merged
merged 18 commits into from
May 27, 2025

Conversation

shortcuts
Copy link
Member

https://algolia.atlassian.net/browse/DI-3785

this implements the bridge methods in algoliasearch to the ingestion api, in order to reduce friction with the transformation pipeline indexing

related: algolia/api-clients-automation#4852

@shortcuts shortcuts self-assigned this May 20, 2025
@shortcuts shortcuts requested review from Haroenv and millotp May 20, 2025 07:22
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

looks like a good idea. Maybe unfortunate that this would be a lot of changes to have as a simple option to saveObject etc. as that would be simpler for the user.

@shortcuts
Copy link
Member Author

looks like a good idea. Maybe unfortunate that this would be a lot of changes to have as a simple option to saveObject etc. as that would be simpler for the user.

The response type differs so we can't really do that without impacting everyone, and I think the discoverability is better with a standalone method rather than an option, wdyt?

@shortcuts shortcuts force-pushed the feat/bridge-transformation-methods branch from a9648f0 to e6ca044 Compare May 22, 2025 11:25
@shortcuts shortcuts requested a review from Haroenv May 22, 2025 14:33
export function createIngestionClient(
options: SearchClientOptions & ClientTransporterOptions & TransformationOptions
): IngestionClient {
if (!options || !options.transformation || !options.transformation.region) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe check that the value is either us or eu

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

idea works for me, details should be approved by ingestion or data team :)

@shortcuts shortcuts requested review from Haroenv and millotp May 26, 2025 07:27
millotp
millotp previously approved these changes May 26, 2025
Copy link
Contributor

@millotp millotp left a comment

Choose a reason for hiding this comment

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

looks good to me !

morganleroi
morganleroi previously approved these changes May 27, 2025
Copy link
Contributor

@millotp millotp left a comment

Choose a reason for hiding this comment

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

gg

@shortcuts shortcuts merged commit 2218b19 into v4 May 27, 2025
4 of 8 checks passed
@shortcuts shortcuts deleted the feat/bridge-transformation-methods branch May 27, 2025 13:22
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.

4 participants