Parameterize timezone variable

[lindhe]

Description

This change parameterizes the timezone variable used across deployments. The new value is global.timezone, following Bitnamis pattern of having a globals object for values that are used across multiple components.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

• [x] I'm aware of the code of conduct.
• [x] I've created logical separate commits and followed the commit message format.
• [ ] I've added relevant test cases.
• [ ] I've added relevant documentation.
• [ ] I've marked translation strings (for relevant code).
• [ ] I've followed the CSS/JS and React guidelines (for relevant code).
• [ ] I've followed the web accessibility guidelines (for relevant code).
• [ ] I've followed the user interface guidelines (for relevant code).
• [ ] I've identified the copyright holder(s) and updated copyright headers for touched files (>15 lines contributions).
• [x] I've NOT included third-party code (copy/pasted source code or new dependencies).

Third-party code

If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

1. You license your contribution under the same terms as the current repository’s license.
2. You agree that you have the right to license your contribution under the current repository’s license.

[plesoun-stein]

it's good idea, I like it and have an improvement.
With invenio as subchart we need to set a global.timezone in main chart. It's prone to errors.

value: {{ .Values.invenio.timezone | default "Europe/Zurich" | quote }}

invenio.timezone may be better location as it may be referenced in main chart or subchart.

And with a default value you can omit setting without error.

[lindhe]

[…] It's prone to errors. […] And with a default value you can omit setting without error.

I agree, we should 100% handle the case when the timezone is unset! Very sloppy of me to not handle that in my original PR, sorry.

I usually prefer using required rather than default. That forces the user to be explicit with their values. It is easier to document; just look in the values.yaml to see what the default value is, rather than 10 different places, which works well with tools like Readme Generator For Helm. Also, I find that using the default function introduces implicit values which may be harder to understand and debug for the end user than the values they are forced to set explicitly. Do you agree?

invenio.timezone may be better location as it may be referenced in main chart or subchart.

Unless I misunderstand what you mean, it is absolutely possible to set global.timezone even when Invenio is included as a subchart! Like this, if I recall correctly:

Chart.yaml

apiVersion: v2
name: foo
description: An umbrella chart that includes subcharts.
dependencies:
  - name: invenio
    version: "1.2.3"
    repository: "https://example.com/charts"

values.yaml

invenio:
  global:
    timezone: Europe/Stockholm

[plesoun-stein]

yes, required is better. definitely.

I'm sorry, i wasn;t accurate with global

You have .Values.global.timezone in your commit. It is use of helm builtin .Values.global

.Values.invenio.global.timezone in your example, It is subkey of invenio definitions.

but both are good option

[lindhe]

I've made the value required now. Do you want me to also drop the global object or not?

A:

# values.yaml
global:
  timezone: "Europe/Zurich"

B:

# values.yaml
timezone: "Europe/Zurich"

Do you want (A) or (B)?

[plesoun-stein]

global object should be better for chart with subcharts.

thanks.

[plesoun-stein]

A variant.

[lindhe]

A variant?

[plesoun-stein]

yes, "A" variant

[lindhe]

Ahaaa, right! Thanks, I get it now. Thought you meant "a variant" and I did not get it at all. 😄

I think it's already the (A) variant. So all good?

[plesoun-stein]

yes. :-)