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

console-api Helm chart #311

Closed
wants to merge 10 commits into from
Closed

console-api Helm chart #311

wants to merge 10 commits into from

Conversation

shimpa1
Copy link

@shimpa1 shimpa1 commented Jan 30, 2025

No description provided.

@shimpa1 shimpa1 requested a review from a team as a code owner January 30, 2025 14:12
Copy link

@ygrishajev ygrishajev left a comment

Choose a reason for hiding this comment

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

looking at console-api templates I don't really understand how it would be different from any upcoming services. Basically if it console-api- prefix is replaces with some var - it would fit most of the cases. Let me know if I'm missing anything pls

- name: console-api-{{ .Values.global.environment }}
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
command: ["node", "dist/server.js"]

Choose a reason for hiding this comment

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

this is set in docker and would be hard to debug if this changes.

Copy link
Author

Choose a reason for hiding this comment

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

OK I'll try a different approach

Copy link
Author

Choose a reason for hiding this comment

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

@ygrishajev I've made a number of changes, would you please take another look, thanks

Copy link

@ygrishajev ygrishajev left a comment

Choose a reason for hiding this comment

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

Thanks a bunch for the changes! Looks much more reusable 💪 Looking forward to test this

charts/console-api/Chart.yaml Outdated Show resolved Hide resolved
charts/console-api/templates/deployment.yaml Show resolved Hide resolved
charts/console-api/templates/doppler-secret.yaml Outdated Show resolved Hide resolved
@shimpa1 shimpa1 closed this Feb 3, 2025
@shimpa1
Copy link
Author

shimpa1 commented Feb 3, 2025

Did another complete revision, will reopen

@shimpa1 shimpa1 deleted the console-api branch February 3, 2025 09:27
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.

2 participants