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

fix(Amazon Seller Partner): Fix daterange in DatetimeBasedCursor an… #55238

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

agarctfi
Copy link
Contributor

@agarctfi agarctfi commented Mar 6, 2025

What

Solves:
https://github.com/airbytehq/oncall/issues/7481 (Source: Amazon Seller Partner - Start Date and End Date not being considered/Incorrect state)

#55170 (Source: amazon-seller-partner - list_financial_events: Reduce max days apart to 14 days)

How

For (Source: Amazon Seller Partner - Start Date and End Date not being considered/Incorrect state)

We found that the end_datetime defaults to now_utc(). This fix changes these instances to check for replication_end_date first before using the default now_utc() value.
Example of one instance:

"{{ format_datetime(config.get('replication_end_date', (now_utc() - duration('PT2M')).strftime('%Y-%m-%dT%H:%M:%SZ') ), '%Y-%m-%dT%H:%M:%SZ') }}"

updated to:

"{{ format_datetime(config['replication_end_date'] if config.get('replication_end_date') else (now_utc() - duration('PT2M')).strftime('%Y-%m-%dT%H:%M:%SZ'), '%Y-%m-%dT%H:%M:%SZ') }}"

This should check for replication_end_date and then default to now_utc () if one is not found. Similar instances were switched to this format for easier readability.

For (Source: amazon-seller-partner—list_financial_events: Reduce the maximum number of days apart to 14 days)

The maximum value listed in the API is 180 days, but in practical use, 14 is the maximum.
After further review, we found these other streams that would face this:
list_financial_event_groups
list_financial_events

This, however, can vary from user to user, so instead. We've implemented an optional configurable value: financial_events_step in the spec. Which is enabled in the manifest by adding:

        step: "{{ config['financial_events_step'] }}"
        cursor_granularity: "PT1S"

This should define the size of each connector's time window to slice the data range between start_datetime and end_datetime for these streams.

In the spec for this, we've set the default value to 14 and used ENUM to prevent users from selecting options outside the hard-coded API limits.

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • [X ] YES 💚
  • NO ❌

…d Added configurable step size for financial events streams
Copy link

vercel bot commented Mar 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 9:53pm

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/amazon-seller-partner labels Mar 6, 2025
@agarctfi
Copy link
Contributor Author

agarctfi commented Mar 6, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (754ed5f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/amazon-seller-partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants