Skip to content

Conversation

@archen2019
Copy link
Contributor

  • Add port number to GF_DATABASE_HOST
  • Read dbname and port number from values file
  • OpenConnectionToDB now detects and supports external DB

Copy link
Collaborator

@cevian cevian left a comment

Choose a reason for hiding this comment

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

Some questions, overall looks good.

{{- $isDefault := not $promEnabled -}}
{{- $root := . -}}
{{- $host := tpl .Values.grafana.timescale.database.host $root -}}
{{- $port := index .Values "timescale-prometheus" "connection" "port" | int -}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems weird to have host in .Values.grafana.timescale.database.host and port in another place. Why don't we just add a key called .Values.grafana.timescale.database.port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, but we can't do it the same way as we did the hostname, since the port is an int. Helm conventions say not to put quotes around ints that arent environment variables, so we will have to just edit the port number in 3 places.

Copy link
Collaborator

@cevian cevian left a comment

Choose a reason for hiding this comment

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

Great work

@archen2019 archen2019 merged commit 85f76be into master Jul 21, 2020
@paulfantom paulfantom deleted the cli branch February 15, 2022 09:15
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.

4 participants