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

[common-library] Add missing features to the common library (NR-281093) #1463

Merged

Conversation

kang-makes
Copy link
Contributor

@kang-makes kang-makes commented Aug 14, 2024

Is this a new chart

No

What this PR does / why we need it:

We thought that some of the features that we need for the super-agent would be awesome to have them at common-library level.

We need the chart to be able to calculate the region (or let the user to specify it) and a new API Key.

Which issue this PR fixes

Special notes for your reviewer:

To read it easier, I would go commit by commit as I added a new feature with its own documentation on each commit.

Checklist

  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [mychartname])

@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch 2 times, most recently from 836cfb5 to 6d2d16e Compare August 14, 2024 14:01
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch 2 times, most recently from 2dba97c to bb8d73e Compare August 14, 2024 14:27
@kang-makes kang-makes marked this pull request as ready for review August 14, 2024 14:28
@kang-makes kang-makes requested a review from a team as a code owner August 14, 2024 14:28
@kang-makes kang-makes requested review from a team August 14, 2024 14:28
@kang-makes kang-makes changed the title [common-library] Add missing features to the common library [common-library] Add missing features to the common library (NR-281093) Aug 14, 2024
Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

The changes look really good to me. I've left some comments to be discussed

library/common-library/templates/_region.tpl Show resolved Hide resolved
library/CHART-TEMPLATE/tests/cl-region.yaml Show resolved Hide resolved
library/common-library/templates/_region.tpl Show resolved Hide resolved
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch from bb8d73e to 48dd6e3 Compare August 19, 2024 14:50
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch from 48dd6e3 to 755db55 Compare August 19, 2024 15:37
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch from 27eb921 to b750ce2 Compare August 21, 2024 07:36
@kang-makes kang-makes requested review from a team and removed request for a team August 22, 2024 10:56
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch from b750ce2 to 15a9b7b Compare August 26, 2024 10:08
@kang-makes kang-makes force-pushed the kang-makes/common-library/add-features-for-super-agent branch from 0486574 to 15ad3fd Compare August 26, 2024 11:34
Copy link
Contributor

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

LGTM

@kang-makes kang-makes requested review from dbudziwojskiNR and a team August 29, 2024 15:31
@dbudziwojskiNR dbudziwojskiNR merged commit e398ef4 into master Aug 29, 2024
11 checks passed
@dbudziwojskiNR dbudziwojskiNR deleted the kang-makes/common-library/add-features-for-super-agent branch August 29, 2024 23:10
kang-makes added a commit that referenced this pull request Sep 23, 2024
#### Is this a new chart
No

#### What this PR does / why we need it:
super-agent chart got the config from a field called `content` that
added any arbitrary data to a config map. This is supposed to control
this configMap and render it using values from Helm's `values.yaml`.

It also adds some protections so a customer is not able to use it on
unsupported environments.

#### Special notes for your reviewer:
This PR is blocked until #1463 is merged.

#### Checklist
- [x] Chart Version bumped
- [x] Variables are documented in the README.md
- [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
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