Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Create and alias index #813

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SoulPancake
Copy link
Contributor

No description provided.

@SoulPancake SoulPancake changed the title feat: Create and aliax index feat: Create and alias ndex Mar 26, 2025
@SoulPancake SoulPancake changed the title feat: Create and alias ndex feat: Create and alias index Mar 26, 2025
@rueian
Copy link
Collaborator

rueian commented Apr 4, 2025

Hi @SoulPancake, I just reviewed it again and left some comments. Could you also write some tests to demonstrate how a user can use this feature?


newIndex := alias + "_v1"
if aliasExists && currentIndex != "" {
// Find the last occurrence of "_v" followed by digits
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about this? @rueian

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM. Please update for the JSONRepository as well.

om/hash.go Outdated
var currentIndex string
infoCmd := r.client.B().FtInfo().Index(alias).Build()
infoResp, err := r.client.Do(ctx, infoCmd).ToMap()
aliasExists := err == nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will be the error returned when the index does not exist? Will it be a redis Nil? I think we should return early here if the err is caused by other reasons, such as network failures.

SoulPancake and others added 2 commits April 9, 2025 11:32
Co-authored-by: Rueian <rueiancsie@gmail.com>
infoCmd := r.client.B().FtInfo().Index(alias).Build()
infoResp, err := r.client.Do(ctx, infoCmd).ToMap()
if err != nil {
if strings.Contains(err.Error(), "Unknown index name") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should work @rueian

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok. please write tests for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will do the changes in JSON as well and then write UTs

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants