Connector SyncJob API spec #2503

jedrazb · 2024-04-11T12:14:57Z

Changes

Added the API spec for endpoints related to connector sync job managements.

l-trotta · 2024-04-15T10:20:16Z

specification/connector_sync_job/_types/SyncJob.ts

+  id: Id
+  indexed_document_count: long
+  indexed_document_volume: long
+  job_type: SyncJobType


can job_type be a SyncJobType or SyncJobType[] enum consistently?

same for trigger_method

In a ConnectorSyncJob representation they are not an array, there is singe value associated with trigger_method and job_type

What I mean is, in this class job_type and trigger_method are enums, while in the classes SyncJobPostRequest.ts and SyncJobListRequest.ts they are Name/Names, which are just strings, can't they be the same enum everywhere?

Ahh I see what you mean! For job_type this is passed as a comma-separated list in the URL param of SyncJobListRequest. I was not sure if defining this as SyncJobType[] would work with URL param list, and I saw Names in other examples of requests that pass comma separated lists in url params.

If the usage of SyncJobType[] and SyncJobTriggerMethod is correct, I can adapt this in the PR.

thanks :) I think it's more coherent this way from the clients perspective, to always provide enums where there's fixed values

I addressed that feedback in the updated PR :)

l-trotta · 2024-04-15T12:57:06Z

I have found a type that was introduced in the previous PR that I'm only now noticing is not matching the server code. Since this new API is using it, could you fix it here?
FilteringConfig -> active and draft should be optional, domain should be removed, it seems like an internal parameter

specification/connector_sync_job/_types/SyncJob.ts

Anaethelion

Left a few comments, found some fields that could be hinted as datetime to enhance deserialization.

specification/connector_sync_job/_types/SyncJob.ts

jedrazb · 2024-04-17T09:02:09Z

I have found a type that was introduced in the previous PR that I'm only now noticing is not matching the server code. Since this new API is using it, could you fix it here?
FilteringConfig -> active and draft should be optional, domain should be removed, it seems like an internal parameter

Yup, there were couple of changes to connector (not sync job) endpoints in 8.14 that were merged after we merged #2367 . I'm planning to followup to adapt the types in a separate PR.

jedrazb · 2024-04-17T09:32:28Z

Sync job endpoints were moved under the connector namespace, I reflected changes in this PR after elastic/elasticsearch#107476 was merged.

Also, I changed Name to more specific allowed enum types (SyncJobTriggerMethod and SyncJobType) in sync_job_post request

specification/connector/_types/SyncJob.ts

github-actions · 2024-04-18T09:00:40Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`connector.check_in`	⚪	Missing test	Missing test
`connector.delete`	⚪	Missing test	Missing test
`connector.get`	⚪	Missing test	Missing test
`connector.last_sync`	⚪	Missing test	Missing test
`connector.list`	⚪	Missing test	Missing test
`connector.post`	⚪	Missing test	Missing test
`connector.put`	⚪	Missing test	Missing test
`connector.sync_job_cancel`	⚪	Missing test	Missing test
`connector.sync_job_delete`	⚪	Missing test	Missing test
`connector.sync_job_get`	⚪	Missing test	Missing test
`connector.sync_job_list`	⚪	Missing test	Missing test
`connector.sync_job_post`	⚪	Missing test	Missing test
`connector.update_api_key_id`	⚪	Missing test	Missing test
`connector.update_configuration`	⚪	Missing test	Missing test
`connector.update_error`	⚪	Missing test	Missing test
`connector.update_filtering`	⚪	Missing test	Missing test
`connector.update_index_name`	⚪	Missing test	Missing test
`connector.update_name`	⚪	Missing test	Missing test
`connector.update_native`	⚪	Missing test	Missing test
`connector.update_pipeline`	⚪	Missing test	Missing test
`connector.update_scheduling`	⚪	Missing test	Missing test
`connector.update_service_type`	⚪	Missing test	Missing test
`connector.update_status`	⚪	Missing test	Missing test

You can validate these APIs yourself by using the make validate target.

jedrazb · 2024-04-18T14:38:05Z

Thank you all for reviewing! I need to include this in the 8.14 release. I still don't see the backport 8.14 label, do I need to do sth specific to make sure it lands in the 8.14 BC?

Anaethelion

LGTM!

github-actions · 2024-04-22T12:56:22Z

The backport to 8.14 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.14 8.14
# Navigate to the new working tree
cd .worktrees/backport-8.14
# Create a new branch
git switch --create backport-2503-to-8.14
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 9f0dc3db0a0f82ef9ebebb276bcfdcaf28c69e6a
# Push it to GitHub
git push --set-upstream origin backport-2503-to-8.14
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.14

Then, create a pull request where the base branch is 8.14 and the compare/head branch is backport-2503-to-8.14.

(cherry picked from commit 9f0dc3d)

* Connector SyncJob API spec (#2503) (cherry picked from commit 9f0dc3d) * Run make contrib

jedrazb added 5 commits April 10, 2024 11:46

Add json spec

95d7e3d

[Connector API] Define sync job API spec

51c021d

Prepare to merge with main

6a35f12

Merge branch 'main' into add-connector-sync-job-api-spec

6d40597

Adjust types, merge with Connector API

7cbe82d

jedrazb requested a review from a team as a code owner April 11, 2024 12:14

github-actions bot added the specification label Apr 11, 2024

l-trotta reviewed Apr 15, 2024

View reviewed changes

specification/connector_sync_job/_types/SyncJob.ts Outdated Show resolved Hide resolved

Anaethelion requested changes Apr 15, 2024

View reviewed changes

specification/connector_sync_job/_types/SyncJob.ts Outdated Show resolved Hide resolved

specification/connector_sync_job/_types/SyncJob.ts Outdated Show resolved Hide resolved

Move sync job endpoints under connector namespace

952436e

Address review feedback

fcc4833

jedrazb requested review from l-trotta and Anaethelion April 17, 2024 09:32

Make types in sync_job_post more specific

ba2f719

l-trotta reviewed Apr 17, 2024

View reviewed changes

specification/connector/_types/SyncJob.ts Show resolved Hide resolved

jedrazb added 2 commits April 18, 2024 10:57

Merge branch 'main' into add-connector-sync-job-api-spec

5a2cf04

Address review feedback

b73b1e2

jedrazb requested a review from l-trotta April 18, 2024 09:00

l-trotta approved these changes Apr 18, 2024

View reviewed changes

Anaethelion approved these changes Apr 22, 2024

View reviewed changes

jedrazb added the backport 8.14 label Apr 22, 2024

jedrazb merged commit 9f0dc3d into main Apr 22, 2024
7 checks passed

jedrazb deleted the add-connector-sync-job-api-spec branch April 22, 2024 12:55

jedrazb added a commit that referenced this pull request Apr 23, 2024

Connector SyncJob API spec (#2503)

00e0bdd

(cherry picked from commit 9f0dc3d)

jedrazb mentioned this pull request Apr 23, 2024

[Backport 8.14] Connector SyncJob API spec (#2503) #2526

Merged

l-trotta pushed a commit that referenced this pull request Apr 23, 2024

[Backport 8.14] Connector SyncJob API spec (#2503) (#2526)

9121888

* Connector SyncJob API spec (#2503) (cherry picked from commit 9f0dc3d) * Run make contrib

Connector SyncJob API spec #2503

Connector SyncJob API spec #2503

Uh oh!

Conversation

jedrazb commented Apr 11, 2024

Changes

Uh oh!

l-trotta Apr 15, 2024

Choose a reason for hiding this comment

Uh oh!

l-trotta Apr 15, 2024

Choose a reason for hiding this comment

Uh oh!

jedrazb Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

l-trotta Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

jedrazb Apr 18, 2024

Choose a reason for hiding this comment

Uh oh!

l-trotta Apr 18, 2024

Choose a reason for hiding this comment

Uh oh!

jedrazb Apr 18, 2024

Choose a reason for hiding this comment

Uh oh!

l-trotta commented Apr 15, 2024

Uh oh!

Uh oh!

Anaethelion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jedrazb commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jedrazb commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 18, 2024

Uh oh!

jedrazb commented Apr 18, 2024

Uh oh!

Anaethelion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Apr 22, 2024

Uh oh!

Uh oh!

jedrazb commented Apr 17, 2024 •

edited

Loading

jedrazb commented Apr 17, 2024 •

edited

Loading