Skip to content

Conversation

ryan109
Copy link

@ryan109 ryan109 commented Jul 18, 2023

Purpose of PR

Parts of the app this will impact

Todos

Additional information

Related links

@ryan109 ryan109 requested a review from gchazot July 18, 2023 17:02

cluster_update_interval = get_transient_cluster_settings(
es_host, 'cluster.info.update.interval',
es_host, ['cluster.info.update.interval'],

Choose a reason for hiding this comment

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

does this being a list or a string depend on the version of ES we're using?

Copy link
Member

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

ok cool

Copy link
Member

@gchazot gchazot left a comment

Choose a reason for hiding this comment

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

Sounds like a sensible change. What's the bug being fixed?


cluster_update_interval = get_transient_cluster_settings(
es_host, 'cluster.info.update.interval',
es_host, ['cluster.info.update.interval'],
Copy link
Member

Choose a reason for hiding this comment

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

@ryan109
Copy link
Author

ryan109 commented Jul 19, 2023

@gchazot specifically this line here, if you pass it in as a string it's splitting on each individual letter instead of list items. I'm very surprised this hasn't come up before tbh...

@gchazot
Copy link
Member

gchazot commented Dec 15, 2023

What's the status on this? It looks like this might be ready to merge and a new release of the tool could be published...?

@ryan109
Copy link
Author

ryan109 commented Dec 15, 2023

Yep it can, I'll deal with it today

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.

3 participants