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

Enable dev mode #76

Merged
merged 10 commits into from
Nov 30, 2023
Merged

Enable dev mode #76

merged 10 commits into from
Nov 30, 2023

Conversation

RushiT0122
Copy link
Contributor

@RushiT0122 RushiT0122 commented Nov 6, 2023

Description of change

  • Dev mode implementation
  • Unitests

Related PRs

Manual QA steps

  1. Verified when refresh token is not provided, access token is not refreshed
  2. Verified when refresh token and client details are provided, access token and refresh token is refreshed on discovery or at the start of sync.
  3. Verified in dev mode, access token is not refreshed
  4. Verified that in above scenarios, discovery and sync runs as expected

Risks

Rollback steps

  • revert this branch

@RushiT0122
Copy link
Contributor Author

FYI... I will raise separate PR for integration tests to implement token chain.

@@ -1,6 +1,6 @@
#!/usr/bin/env python3
import singer
from singer import utils
from singer import utils as _utils
Copy link
Member

Choose a reason for hiding this comment

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

does this import create a conflict with the tap utils.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does because of which I have used an alias here.

Copy link
Contributor Author

@RushiT0122 RushiT0122 Nov 8, 2023

Choose a reason for hiding this comment

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

It was causing issue with below import statement in client.py#L8

from tap_typeform.utils import write_config

@@ -138,7 +189,7 @@ def get_page_size(self, config):
if page_size is None:
return
if ((type(page_size) == int or type(page_size) == float) and (page_size > 0)) or \
(type(page_size) == str and page_size.replace('.', '', 1).isdigit() and (float(page_size) > 0) ):
(type(page_size) == str and page_size.replace('.', '', 1).isdigit() and (float(page_size) > 0)):
Copy link
Member

@cngpowered cngpowered Nov 8, 2023

Choose a reason for hiding this comment

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

we can simplify this by a small block instead of using a function to parse page_size in the init
eg:

try:
    page_size = int(config.get('page_size') or 0)
    if page_size > 0:
        self.page_size = page_size
        self.form_page_size = min(self.form_page_size, self.page_size)
    else:
        raise ValueError("Negative or invalid ....")
 except (ValueError, TypeError) as err:
    LOGGER.info("Invalid Value: %s, for page_size", config.get('email_activity_date_window', 0))
    raise from err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unintentional refactoring change without any logical change. I would not prefer to cause any regression because of new changes.

Copy link
Member

Choose a reason for hiding this comment

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

okay

Comment on lines 2 to +9
import backoff
import singer

from datetime import timedelta
from singer.utils import now
from requests.exceptions import ChunkedEncodingError, Timeout, ConnectionError
from tap_typeform.utils import write_config

Copy link
Member

Choose a reason for hiding this comment

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

from datetime import timedelta
import backoff
from requests.exceptions import ChunkedEncodingError, Timeout, ConnectionError
import singer
from singer.utils import now
from tap_typeform.utils import write_config

i suggest keeping standard imports first

tap_typeform/client.py Outdated Show resolved Hide resolved
@RushiT0122 RushiT0122 removed the request for review from luandy64 November 28, 2023 09:59
* add token chaining in integration tests

---------

Co-authored-by: RushiT0122 <[email protected]>
@RushiT0122 RushiT0122 merged commit 7030955 into master Nov 30, 2023
3 checks passed
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