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

check for empty breadcrumb the right way #58

Merged
merged 2 commits into from
Nov 5, 2019

Conversation

nick-mccoy
Copy link
Contributor

Description of change

When validating the state, check for an empty breadcrumb by checking for an empty list instead of an empty tuple.

Manual QA steps

Had a client that was stuck in an infinite loop since they de-selected the stream that was bookmarked as currently_syncing. Was able to verify that this fix takes them out of the looping behavior.

Risks

None since this function was not working correctly before

Rollback steps

Revert this change

Copy link
Contributor

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

This does look like the correct behavior, since the state and catalog will be loaded from JSON, which doesn't have the concept of a tuple. Unit tests should be updated to reflect this.

tap_marketo/__init__.py Show resolved Hide resolved
@nick-mccoy nick-mccoy merged commit d5ca1ed into master Nov 5, 2019
@nick-mccoy nick-mccoy deleted the fix/empty-breadcrumb-check branch November 5, 2019 22:43
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.

2 participants