Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Conversation

@sezruby
Copy link
Collaborator

@sezruby sezruby commented Oct 13, 2020

What is the context for this pull request?

Fixes #196

What changes were proposed in this pull request?

Instead of using the current spark.config, this PR gets numBuckets and lineage column config from previous index entry at refresh and use it for refresh.

Does this PR introduce any user-facing change?

Fixes a buggy behavior.

How was this patch tested?

Unit test

@sezruby sezruby self-assigned this Oct 13, 2020
@sezruby sezruby added the bug Something isn't working label Oct 13, 2020
@sezruby sezruby added this to the October 2020 milestone Oct 13, 2020
@sezruby
Copy link
Collaborator Author

sezruby commented Oct 13, 2020

@imback82 What if a user wants to change the configs - with full refresh mode? Should we consider that case also?

@imback82
Copy link
Contributor

@imback82 What if a user wants to change the configs - with full refresh mode? Should we consider that case also?

Let's not support it for now (the user can create a new index with a different number of buckets if needed - since refreshing full is like creating a new one). We can introduce a "overwrite" config later if needed.

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

Few minor comments, but the PR looks fine to me.

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

One minor comment, but LGTM, thanks @sezruby!

@imback82 imback82 changed the title Use configs of previousLogEntry at refresh Use configs of previous LogEntry for refresh action Oct 13, 2020
@imback82 imback82 merged commit 7542735 into microsoft:master Oct 13, 2020
@sezruby sezruby deleted the configfix branch October 13, 2020 06:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refresh index should pick bucket num and lineage column configs from previous entry instead of session config

2 participants