这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Conversation

@ayashjorden
Copy link

Tested '/' functionality as concrete ES index name signaling (without Bosun adding date suffix), works great.

Review on Reviewable

Copy link
Contributor

Choose a reason for hiding this comment

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

This and the line below can be reduced to if strings.HasSuffix(r.IndexRoot, "/").

@ayashjorden
Copy link
Author

@mjibson, (as I'm new to GO) Should I reduce it?

@madelynnblue
Copy link
Contributor

Yes, you should. But I'm a bit confused about the intended behavior. That will test if just the last character is a "/", not if the entire string has value "/". Which do you want?

@ayashjorden
Copy link
Author

As you can see in #974 , the intended behavior is to enable the user to signal Bosun 'just use this index name and don't add date suffix'.

The context behind this is that ES indices 'costs' quite a lot in terms of memory and cpu. there are use-cases such as the one that initiated this PR that the size of data is <5MB per day so there is no reason to create new indices every day. also there's the fact that olivere/elastic client (that Bosun is using to interact with ES) query's indices by index name, and doesn't include index aliases (which you can think of sym-links from 'myindex-2015.05.31' to 'myindex-global').

Will the 'strings.HasSuffix(r.IndexRoot, "/")' will handle nil vals gracefully?

Hope that the above is clear enoungh,
Yarden

@madelynnblue
Copy link
Contributor

Strings can't be nil, but they can be empty, and HasSuffix handles that correctly.

@madelynnblue
Copy link
Contributor

@kylebrandt, could you review this?

@kylebrandt
Copy link
Member

+1

madelynnblue added a commit that referenced this pull request Jun 22, 2015
@madelynnblue madelynnblue merged commit 0e46b15 into bosun-monitor:master Jun 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants