Skip to content

Conversation

@dylanlott
Copy link
Contributor

Description

Returns an ErrSnapshotInterrupt when a snapshot is stopped before it's finished.
Creates the ErrSnapshotComplete variable in anticipation of creating the CombinedIterator

Addresses #22 (comment)

Quick checks:

  • I have followed the Code Guidelines.
  • There is no other pull request for the same update/change.
  • I have written unit tests.
  • I have made sure that the PR is of reasonable size and can be easily reviewed.

@dylanlott dylanlott mentioned this pull request Apr 6, 2022
4 tasks
// Teardown attempts to gracefully teardown the iterator.
func (s *SnapshotIterator) Teardown(ctx context.Context) error {
s.rows.Close()
if commitErr := s.tx.Commit(ctx); commitErr != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a dumb question, but why do we call s.tx.Commit() here at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

Simply to end the transaction.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I think I need a break.:D I was thinking this is the source teardown, but it's the iterator teardown, and actually a different thing in mind because of that.

@dylanlott dylanlott marked this pull request as ready for review April 7, 2022 20:04
@dylanlott dylanlott requested a review from a team as a code owner April 7, 2022 20:04
Base automatically changed from dylan/snapshot-iterator to main April 8, 2022 16:09
@dylanlott dylanlott requested a review from lovromazgon April 8, 2022 17:27
Copy link
Contributor

@lovromazgon lovromazgon left a comment

Choose a reason for hiding this comment

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

🙌

@dylanlott dylanlott merged commit 873fc9e into main Apr 8, 2022
@dylanlott dylanlott deleted the dylan/teardown branch April 8, 2022 18:14
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.

4 participants