Skip to content

Set Default start_date to 3 Years Ago for actions and actionUpdates#33

Merged
rdeshmukh15 merged 9 commits into
masterfrom
TDL-26510/fix-older-startdate-error
Dec 16, 2024
Merged

Set Default start_date to 3 Years Ago for actions and actionUpdates#33
rdeshmukh15 merged 9 commits into
masterfrom
TDL-26510/fix-older-startdate-error

Conversation

@rdeshmukh15

@rdeshmukh15 rdeshmukh15 commented Sep 26, 2024

Copy link
Copy Markdown
Member

Description of change

  1. According to the impact document at: https://integrations.impact.com/impact-brand/changelog/changes-to-actions-and-action-updates, new changes were implemented on August 5, 2024. One of these changes states that queries for Actions or ActionUpdates cannot have a StartDate that is more than 3 years old.

    As a result of this restriction, customers began encountering errors when the start_date specified in their configuration
    is older than 3 years. The error message is as follows:

    `2024-09-09 15:26:43,856Z tap - ERROR ERROR 400: {"Status":"ERROR","Message":"Parameter 'ActionDateStart' 
     cannot be more than 3 years in the past."}, REASON: Bad Request`
    

    Hence, as part of fix, added a default start_date to 3 years ago when start_date is more than 3 years old.

  2. remove implementation of clicks stream as it is deprecated #34

  3. Date window implementation for actions and actionUpdates #35

  4. Bump requests from 2.31.0 to 2.32.0 #28

Manual QA steps

  • Tested the changes on customer connection and those are working fine.

Risks

  • low

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

@rdeshmukh15 rdeshmukh15 changed the title if start date is older than 3 years for actions and actionupdates mak… make start_date default to back to 3 years for actions and actionUpdates Sep 26, 2024
@rdeshmukh15 rdeshmukh15 changed the title make start_date default to back to 3 years for actions and actionUpdates Set Default start_date to 3 Years Ago for actions and actionUpdates Sep 26, 2024

@RushiT0122 RushiT0122 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

IMO, we should fail the discovery if date is older than 3 years.

Comment thread tap_impact/sync.py
@rdeshmukh15

Copy link
Copy Markdown
Member Author

IMO, we should fail the discovery if date is older than 3 years.

This rule of having start_date only upto 3 years is for Actions and ActionUpdates.

rdeshmukh15 and others added 4 commits October 7, 2024 17:05
* remove implementation of clicks as it is deprecated

* pylint fix

* Date window implementation for actions and actionUpdates (#35)

* date window of 45 days implemented for actions and actionUpdates

* refactoring

* change default windowing range to 45 days for actions and action_updates

* refactoring

* change in comment

---------

Co-authored-by: “rdeshmukh15” <“rutuja.deshmukh@qlik.com”>

---------

Co-authored-by: “rdeshmukh15” <“rutuja.deshmukh@qlik.com”>
updated-dependencies:
- dependency-name: requests
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rutuja Deshmukh <107538720+rdeshmukh15@users.noreply.github.com>
- Updates the requests to latest version
Comment thread tap_impact/sync.py
Comment thread tap_impact/sync.py Outdated
Comment thread tests/unittests/test_default_start_date.py Outdated
Comment thread tests/unittests/test_split_date_range.py Outdated
Comment thread tap_impact/sync.py
BASE_URL = 'https://api.impact.com'
# Window size implemented for actions and actionUpdates streams
# as startDate and EndDate can just be 45 days apart
DEFAULT_WINDOW_SIZE = 45

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

any specific reason to keep it 45 days? rather than a standard 1 month or 30 days date window?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the PR referenced in description which tells about this in detail: #35

@rdeshmukh15 rdeshmukh15 merged commit c8d5510 into master Dec 16, 2024
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.

3 participants