Data Source Connectors: Propagate error and misc by sats-23 · Pull Request #1284 · IBM/project-ai-services

sats-23 · 2026-08-19T04:45:35Z

Cleanup removed failed_files from init_schema
Removed last_sync_error from connectors table
Async auth validation on every PUT/POST call on connectors which updates the connectors table
--This error is never overwritten by a sync tick failure error message (highlighting root cause being wrong creds)
--Only correct creds via another PUT request can clear the previous auth failure message
Updated proposal
Increased UT coverage
Used the secret as per catalog deployment
Rebrand connectors response into simple ID and name

sats-23 · 2026-08-19T05:58:29Z

    return settings.digitize.connector.encryption_key_path


+async def _probe_connector_credentials(


This is a makeshift solution to async update the DB (for failed creds).
This still does not provide direct validation to the end-user when a PUT/POST on connector is done

I am not fully convinced this is really needed, because in case of POST the immediate sync operation will do this and report this failure anyway.
And for PUT this would be done in next sync which would be 5 mins later.
But somehow after seeing this, I m thinking why not trigger the sync immediately after PUT as well like POST, if someone updates the connector, I feel they would like the sync to happen immediately wdyt?
If we do that, this probing is not needed.

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

dharaneeshvrd · 2026-08-21T06:59:19Z

    return settings.digitize.connector.encryption_key_path


+async def _probe_connector_credentials(


I am not fully convinced this is really needed, because in case of POST the immediate sync operation will do this and report this failure anyway.
And for PUT this would be done in next sync which would be 5 mins later.
But somehow after seeing this, I m thinking why not trigger the sync immediately after PUT as well like POST, if someone updates the connector, I feel they would like the sync to happen immediately wdyt?
If we do that, this probing is not needed.

dharaneeshvrd · 2026-08-21T07:03:17Z

+                f"Scheduler registration failed for {connector_id!r}: {sched_exc}",
+                exc_info=True,
            )
            raise


These kind of empty raise statements needs to be updated with a prefix message saying on what operation and situation, this specific exception is raised. Please take care of all the plain raise statements wrt to the connector changes added.

This applies to other endpoint handlers as well.
Also please ensure to log them before raising

@manalilatkar @Niharika0306
I think similar behaviour need to be taken care in extract and translate service as well.
Please see whether you can address them in your upcoming PRs

dharaneeshvrd · 2026-08-21T07:06:27Z

+    Inherits from str so values can be compared directly against DB strings.
+    """
+
+    CREDENTIAL_ERROR_MSG = "Authentication failed: unable to connect with the provided credentials"


this should be used on auth issues faced during sync operation as well

dharaneeshvrd · 2026-08-21T07:08:31Z

    total_files      INTEGER     NOT NULL DEFAULT 0,
    new_files        INTEGER     NOT NULL DEFAULT 0,
    removed_files    INTEGER     NOT NULL DEFAULT 0,
-    failed_files     INTEGER     NOT NULL DEFAULT 0,


can you help me remember why are we removing failed_files?

dharaneeshvrd · 2026-08-21T07:13:10Z

                if total_files is not None:
                    values["total_files"] = total_files
-                if error is not None:
+                if error is not _UNSET:


can you not simply do this by removing this if?
what ever is passed for error arg needs to be set as is.
So when an error exists currently, if its cleared in next run, the error passed here would be none, which can be set as is. wdyt?

dharaneeshvrd

It would be great, if you can produce possible error cases and how the connector and sync log's error fields looks like.

dharaneeshvrd · 2026-08-21T08:25:41Z

@@ -129,7 +180,7 @@ async def create_connector(body: ConnectorCreateRequest):
        # id or name already exists


I think we also discussed to log the exceptions before raising, can you please take care in all the occurrences?

dharaneeshvrd · 2026-08-21T08:29:41Z

    if not found:
        return False
-    db_manager.update_connector_after_sync(connector_id, status=status, last_sync_at=now)
+    db_manager.update_connector_after_sync(


It's better to rephrase the error message of a last sync when we update it on connector to depict that it s from the last sync.

dharaneeshvrd · 2026-08-21T08:31:40Z

    if batch_failed:
        raise RuntimeError(
            f"One or more batches failed to ingest for connector {connector_id!r}; "
            "connector marked as out of sync"


This is going to be a user facing error message, can we please rephrase accordingly?
Something like: One or more documents failed to sync. See more details on digitize jobs <f"Connector-{connector_name}-{sync_seq}-*>

dharaneeshvrd · 2026-08-21T08:32:55Z

+                f"Scheduler registration failed for {connector_id!r}: {sched_exc}",
+                exc_info=True,
            )
            raise


This applies to other endpoint handlers as well.
Also please ensure to log them before raising

sats-23 added the squad/usecases label Aug 19, 2026

sats-23 commented Aug 19, 2026

View reviewed changes

sats-23 requested review from dharaneeshvrd and manju956 August 19, 2026 06:13

sats-23 marked this pull request as ready for review August 19, 2026 06:13

sats-23 force-pushed the DCImpl7 branch from ac46e2e to 9d8916b Compare August 19, 2026 14:21

sats-23 added 10 commits August 21, 2026 10:28

Remove last_sync_error and update error when needed

a4ecb20

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Probe connection details and update table

96c88c7

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Override sync error only on non-auth failure

67d5568

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Cleanup removed var

2d5cf13

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Update Proposal

33ea09b

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Increase UT coverage

5b574b1

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Enhance logs

7315061

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Add marker to avoid warning

51fc553

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Fetch DB encryption key from secrets

7049512

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

Rebrand connectors as simple id and name

695715b

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

sats-23 force-pushed the DCImpl7 branch from e7418d0 to 695715b Compare August 21, 2026 04:59

Remove unused var

22e9438

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

sats-23 force-pushed the DCImpl7 branch 2 times, most recently from 72685cb to 22e9438 Compare August 21, 2026 06:10

Fix tests

1ecc696

Signed-off-by: Sathvik <Sathvik.S@ibm.com>

dharaneeshvrd reviewed Aug 21, 2026

View reviewed changes

		return settings.digitize.connector.encryption_key_path


		async def _probe_connector_credentials(

		@@ -129,7 +180,7 @@ async def create_connector(body: ConnectorCreateRequest):
		# id or name already exists

Uh oh!

Conversation

sats-23 commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dharaneeshvrd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sats-23 commented Aug 19, 2026 •

edited

Loading