Skip to content

fix: show missing retention policies - #6145

Merged
jansimonb merged 3 commits into
masterfrom
fix/show-retention-policies
Aug 12, 2025
Merged

fix: show missing retention policies#6145
jansimonb merged 3 commits into
masterfrom
fix/show-retention-policies

Conversation

@jansimonb

@jansimonb jansimonb commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

Fix parsing result of SHOW RETENTION POLICIES. Enterprise v1.12.1 newly returns 2 new columns in the result:

  • futureWriteLimit
  • pastWriteLimit

These 2 new columns were unknown to chronograf yet, so the result parsing had to be fixed.

Now the code supports 2 result formats:

  • 5-column format: [name, duration, shardGroupDuration, replicaN, default]
  • 7-column format: [name, duration, shardGroupDuration, replicaN, futureWriteLimit, pastWriteLimit, default]

Additionally, error logging was added to make it easier to detect similar issues in the future.

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Well-formatted commit messages
  • Rebased/mergeable
  • Tests pass
  • Any changes to etc/Dockerfile_build have been pushed to DockerHub, and the changes have been added to .circleci/config.yml
  • swagger.json updated (if modified Go structs or API)
  • Sign CLA (if not already signed)

@jansimonb
jansimonb marked this pull request as ready for review August 11, 2025 10:59

@bednar bednar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The implementation looks good. Please add a few tests similar to:

func Test_showResults_Permissions(t *testing.T) {

@bednar bednar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 🚀

@karel-rehor

Copy link
Copy Markdown
Contributor

1.Rebuilt Chronograf in circle-ci Chronograf 202508121131~92117c5 (git: 92117c55a2571368da12f66924ab14d37d20f38e) then downloaded and ran it on local Ubuntu 22.0.4.

2.Started a local dockerized influxdb:1.8 and connected it to the running chronograf.

  1. Tested it via CLI with $ influx v1 shell --host http://localhost:18086 and then successfully looked up retention policies. e.g.
> SHOW RETENTION POLICIES ON mydb
> SHOW RETENTION POLICIES ON _internal
  1. Then checked the same results in chronograf > influxdb admin. Retention policies render successfully.
Screenshot from 2025-08-12 17-30-03

Conclusion, looks corrrect.

@karel-rehor karel-rehor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Short manual test result is in comment in conversation. Looks correct to me. 🚴 🏁

@jansimonb
jansimonb merged commit 0291d50 into master Aug 12, 2025
3 checks passed
@jansimonb
jansimonb deleted the fix/show-retention-policies branch August 12, 2025 16:08
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