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.

Sign up for GitHub

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

Jump to bottom

Fix the stage diff algorithm #1031

Merged
merged 3 commits into from
May 2, 2024
Merged

Fix the stage diff algorithm #1031

merged 3 commits into from
May 2, 2024

Conversation

sfc-gh-cgorrie
Copy link
Contributor

@sfc-gh-cgorrie sfc-gh-cgorrie commented Apr 30, 2024 •
edited
Loading

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated automated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • I've confirmed that my changes are working by executing CLI's commands manually.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the release notes.
  • I've described my changes in the section below.

Changes description

Our stage diff algorithm depended on the stage plugin returning a SnowflakeCursor, but it started returning a DictCursor which has a different contract for fetchall(). Unfortunately, mypy did not catch this because DictCursor is a subclass of SnowflakeCursor.

These changes were split off from #933.

@sfc-gh-cgorrie sfc-gh-cgorrie requested review from a team as code owners April 30, 2024 20:22
sfc-gh-bdufour
sfc-gh-bdufour previously approved these changes Apr 30, 2024
View reviewed changes
sfc-gh-bdufour
sfc-gh-bdufour previously approved these changes Apr 30, 2024
View reviewed changes
sfc-gh-bdufour
sfc-gh-bdufour previously approved these changes Apr 30, 2024
View reviewed changes
@sfc-gh-cgorrie sfc-gh-cgorrie force-pushed the fix-stage-diff-algo branch from d1eed10 to e72ed8e Compare May 1, 2024 00:38
@sfc-gh-cgorrie sfc-gh-cgorrie changed the base branch from main to gbloom-SNOW-1238239-deploy-files-argument May 1, 2024 00:38
sfc-gh-bgoel
sfc-gh-bgoel previously approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-bgoel sfc-gh-bgoel left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for the bugfix.

@sfc-gh-cgorrie sfc-gh-cgorrie force-pushed the fix-stage-diff-algo branch from e72ed8e to e5aeaac Compare May 1, 2024 17:02
@sfc-gh-cgorrie sfc-gh-cgorrie changed the base branch from gbloom-SNOW-1238239-deploy-files-argument to main May 1, 2024 17:02
@sfc-gh-cgorrie sfc-gh-cgorrie dismissed sfc-gh-bgoel’s stale review May 1, 2024 17:02

The base branch was changed.

@sfc-gh-cgorrie sfc-gh-cgorrie enabled auto-merge (squash) May 2, 2024 14:24
@sfc-gh-cgorrie sfc-gh-cgorrie merged commit bd8845d into main May 2, 2024
11 checks passed
@sfc-gh-cgorrie sfc-gh-cgorrie deleted the fix-stage-diff-algo branch May 2, 2024 15:38
sfc-gh-sichen pushed a commit that referenced this pull request Oct 17, 2024
@sfc-gh-cgorrie
Fix the stage diff algorithm (#1031)
d190f7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-bdufour sfc-gh-bdufour sfc-gh-bdufour approved these changes

@sfc-gh-jsikorski sfc-gh-jsikorski sfc-gh-jsikorski approved these changes

@sfc-gh-bgoel sfc-gh-bgoel sfc-gh-bgoel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sfc-gh-cgorrie @sfc-gh-bgoel @sfc-gh-jsikorski @sfc-gh-bdufour