Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix sectionless items edge case #303

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

santilococo
Copy link
Contributor

@santilococo santilococo commented Dec 20, 2024

This PR fixes an edge case bug introduced by #300.

The edge case is explained in #300 (comment)

The issue occurs because, for sectionless items (keys) to work, they must be at the top. However, the updated grafana template did not guarantee this unless the user explicitly placed them at the top of the grafana_ini variable. This approach is suboptimal, as the template should handle this automatically. This PR fixes the issue by managing this edge case first before processing other cases.

Example:

grafana_ini:
  security:
    admin_user: "admin"
  
  instance_name: "grafana.com"

With the previous PR, this would generate:

[security]
admin_user = admin
admin_email = admin@localhost

instance_name = grafana.com

This is incorrect, as instance_name becomes part of the security section and won't be recognized by grafana.

With this PR, the output is correctly generated as:

instance_name = grafana.com

[security]
admin_user = admin
admin_email = admin@localhost

@intermittentnrg
Copy link
Contributor

Looking good!

@voidquark
Copy link
Collaborator

Nice find! 🚀

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