Sac 30240 tap codat standardize state by akkumar-qlik · Pull Request #29 · singer-io/tap-codat

akkumar-qlik · 2026-02-23T01:12:13Z

Description of change

https://qlik-dev.atlassian.net/browse/SAC-30240

Added changes related to standardize state and bookmark update as per the discussion in the ticket:
The company-scoped bookmark pattern is incompatible with both target-qlik and stitch-menagerie-service. They are expecting the keys under the bookmarks to be the tap_stream_id.

tap-codat's Current Implementation:

{
  "bookmarks": {
    "stream_name.companyId": {
      "field": "modifiedDate",
      "last_record": "2024-01-01T00:00:00Z"
    }
  }
}

How data gets nested under that shouldn’t matter to either the target or menagerie, so something like this would be acceptable:

{
  "bookmarks": {
    "companies": {
      "{COMPANY_ID_1}": {...},
      "{COMPANY_ID_2}": {...}
    }
  }
}

As far as the custom bookmark structure goes, as long as the entire bookmark gets cleared out on reset, the target and menagerie should work as expected.

// BAD
{
  "bookmarks": {
    "companies": {
      "{COMPANY_ID_1}": {},
      "{COMPANY_ID_2}": null
    }
  }
}
// GOOD
{
  "bookmarks": {
    "companies": {}
  }
}
// ALSO GOOD
{
  "bookmarks": {}
}

* update circle config to use uv * fix yml * validate json schemas

* bump deps * changelog update

…ndardize-state

Copilot

Pull request overview

This PR refactors the state management system in tap-codat to use a properly nested bookmark structure organized by stream and company ID, replacing the previous composite key format. The changes improve state organization and align with Singer tap conventions by scoping bookmarks per company within each stream.

Changes:

Refactored state bookmark structure from bookmarks[stream.companyId] to bookmarks[stream][companyId] for proper hierarchical organization
Added comprehensive test suite for state management functions with 100% coverage of edge cases
Modernized CI/CD pipeline from virtualenv/pip/nose to uv/pytest with Python 3.12
Updated dependencies (singer-python 6.1.1→6.7.0, requests 2.32.4→2.32.5) and bumped version to 0.5.4

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tap_codat/state.py	Updated `get_last_record_value_for_table` and `incorporate` functions to support company-scoped bookmarks with nested dictionary structure
tap_codat/streams.py	Modified `capture_state` class to pass `stream_id` and `company_id` separately instead of composite string
tap_codat/context.py	Removed unused offset-related methods (`get_offset`, `set_offset`, `clear_offsets`)
test/test_state.py	Added comprehensive test suite covering all state management scenarios including edge cases and validation rules
setup.py	Bumped version to 0.5.4 and updated dependency versions
.circleci/config.yml	Modernized CI pipeline to use uv package manager, pytest testing framework, and Python 3.12
CHANGELOG.md	Minor formatting fix (trailing newline)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T05:43:46Z

+        new_state['bookmarks'][table] = {}
+
+    current_value = new_state['bookmarks'].get(table, {}).get(company_id, {}).get('last_record')
+    if current_value is None or current_value < parsed:


The comparison current_value < parsed on line 36 compares strings lexicographically. While ISO 8601 datetime strings in the format 'YYYY-MM-DDTHH:MM:SSZ' do sort correctly lexicographically, this relies on the format being consistent. If the format changes or if timezone offsets are included (e.g., '+00:00' vs 'Z'), the comparison could produce incorrect results. Consider parsing both values to datetime objects before comparison, or add a comment explaining that lexicographic comparison is intentional and safe for this specific ISO 8601 format.

Copilot · 2026-02-23T05:43:46Z

    if 'bookmarks' not in new_state:
        new_state['bookmarks'] = {}

-    if(new_state['bookmarks'].get(table, {}).get('last_record') is None or
-       new_state['bookmarks'].get(table, {}).get('last_record') < value):
-        new_state['bookmarks'][table] = {
+    if table not in new_state['bookmarks']:
+        new_state['bookmarks'][table] = {}
+
+    current_value = new_state['bookmarks'].get(table, {}).get(company_id, {}).get('last_record')
+    if current_value is None or current_value < parsed:
+        new_state['bookmarks'][table][company_id] = {


This change modifies the state bookmark structure from bookmarks[stream.companyId] to bookmarks[stream][companyId], which is a breaking change. Existing state files using the old format will not be recognized, causing the tap to resync all data from the start_date. Consider either: (1) adding migration logic to convert old state format to new format, (2) documenting this as a breaking change in CHANGELOG.md with clear upgrade instructions, or (3) adding backward compatibility to read from both old and new formats during a transition period.

agree we need to mention this in change log as breaking change.

atttiwari · 2026-02-23T07:30:41Z

+        new_state['bookmarks'][table][company_id] = {
            'field': field,
            'last_record': parsed,
        }


Although as per the @skuttleman comments. we can keep the nested key value data, but it should not be null or {}, we need to check this before finalizing the state file. and should add one check for this.

Updated the code to do sanity before finalizing the state file

skuttleman · 2026-02-23T14:46:00Z

    def get_max(self):
-        company_stream = "{}.{}".format(self.stream_id, self.company_id)
-        state_dt = get_last_record_value_for_table(self.ctx.state, company_stream)
+        state_dt = get_last_record_value_for_table(self.ctx.state, self.stream_id, self.company_id)


I think we'll need to support retrieving state values in the old format to keep the tap compatible with any existing connections.

Updated the code

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

RushiT0122 · 2026-03-02T12:39:46Z



 def sync(ctx):
+    sanitize_bookmarks(ctx.state)


Sanitization is needed after sync is completed than at the begenning.

RushiT0122 · 2026-03-02T12:46:20Z

+    for table in list(bookmarks.keys()):
+        _sanitize_stream_bookmark(state, table)


Suggested change

for table in list(bookmarks.keys()):

_sanitize_stream_bookmark(state, table)

for stream_name in list(bookmarks.keys()):

_sanitize_stream_bookmark(state, stream_name)

RushiT0122 · 2026-03-02T13:32:35Z

    install_requires=[
-        "singer-python==6.1.1",
-        "requests==2.32.4",
+        "singer-python==6.7.0",


Suggested change

"singer-python==6.7.0",

"singer-python==6.8.0",

RushiT0122 · 2026-03-02T13:50:45Z

 setup(
    name="tap-codat",
-    version="0.5.3",
+    version="0.5.4",


Versioning is not in sync with CHANGELOG, fix it.

Co-authored-by: Rushikesh Todkar <98420315+RushiT0122@users.noreply.github.com>

skuttleman and others added 3 commits August 7, 2025 10:40

SAC-28246: update circle config to use uv (#25)

9f54842

* update circle config to use uv * fix yml * validate json schemas

bump dependencies (#28)

a03939b

* bump deps * changelog update

Added changes related to bookmark and Standardize State

002264b

akkumar-qlik requested review from RushiT0122 and atttiwari February 23, 2026 01:12

akkumar-qlik self-assigned this Feb 23, 2026

akkumar-qlik added 2 commits February 23, 2026 06:44

Merge branch 'SAC-28823-metadata-to-tap' into SAC-30240-tap-codat-sta…

0e09310

…ndardize-state

Added minor changes

8942c87

RushiT0122 requested a review from Copilot February 23, 2026 05:39

Copilot started reviewing on behalf of RushiT0122 February 23, 2026 05:39 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

atttiwari reviewed Feb 23, 2026

View reviewed changes

Added sanity for empty value company data

d70983d

skuttleman reviewed Feb 23, 2026

View reviewed changes

akkumar-qlik and others added 3 commits February 24, 2026 10:27

Update tap_codat/state.py

f4e614e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update .circleci/config.yml

805b555

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Added minor changes

7196cec

RushiT0122 reviewed Mar 2, 2026

View reviewed changes

RushiT0122 requested changes Mar 2, 2026

View reviewed changes

Added review comment changes

5d1a55c

akkumar-qlik requested a review from RushiT0122 March 3, 2026 11:15

akkumar-qlik added 2 commits March 10, 2026 15:54

Added version

f4cfe4c

Added PR link

497e25f

RushiT0122 reviewed Mar 10, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread setup.py Outdated

akkumar-qlik and others added 2 commits March 10, 2026 19:19

Update CHANGELOG.md

75fd9f8

Co-authored-by: Rushikesh Todkar <98420315+RushiT0122@users.noreply.github.com>

Update setup.py

2de0df3

Co-authored-by: Rushikesh Todkar <98420315+RushiT0122@users.noreply.github.com>

akkumar-qlik requested a review from RushiT0122 March 10, 2026 21:49

Added minor changes

a274b31

RushiT0122 approved these changes Mar 11, 2026

View reviewed changes

		for table in list(bookmarks.keys()):
		_sanitize_stream_bookmark(state, table)

Conversation

akkumar-qlik commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of change

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

atttiwari Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

akkumar-qlik commented Feb 23, 2026 •

edited

Loading

atttiwari Feb 23, 2026 •

edited

Loading