Skip to content

Conversation

tim-quix
Copy link
Contributor

@tim-quix tim-quix commented Jul 1, 2025

Extracted from PR quixio/quix-samples#620 (made changes to be more in line with typical Source pattern).

Comment on lines +37 to +46
**{
"host": host,
"port": port,
"user": user,
"password": password,
"database": database,
"table": table,
"snapshot_host": snapshot_host or host,
"state_dir": state_dir,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it doesn't need to be a dict

Suggested change
**{
"host": host,
"port": port,
"user": user,
"password": password,
"database": database,
"table": table,
"snapshot_host": snapshot_host or host,
"state_dir": state_dir,
}
host=host,
port=port,
user=user,
password=password,
database=database,
table=table,
snapshot_host=snapshot_host or host,
state_dir=state_dir,

"Initial snapshot is enabled and not yet completed - performing snapshot..."
)

if not self.is_snapshot_completed() or self.force_snapshot:
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a bit of redundant logic here because is_snapshot_completed also checks self.force_snapshot. If you ask me, the method should not check force_snapshot, it should only check actual completeness, not if the snapshot should be done forcefully.

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