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

[Improvement][AMORO-3178]: Change the default configuration of values.yaml to improve user experience #3179

Merged
merged 4 commits into from
Oct 12, 2024

Conversation

upczsh
Copy link
Contributor

@upczsh upczsh commented Sep 6, 2024

Why are the changes needed?

Close #3178

Brief change log

  • as title

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

Open the prometheus annotation in values ​​and helm deployment will report an error.

image

Apply this patch and open prometheus annotation for helm deployment.

image

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@@ -120,7 +120,7 @@ spark distribution package will be installed to here*/ -}}
{{- if .Values.plugin.metricReporters }}
{{- if .Values.plugin.metricReporters.prometheusExporter.enabled }}
- name: prometheus
containerPort: {{ .Values.plugin.metricReporters.prometheusExporter.port }}
Copy link
Contributor

Choose a reason for hiding this comment

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

if have anothor properties,not just only port.maybe can use map set of this config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @czy006 , is this what you said?

prometheusExporter:

  name: prometheus-exporter

  enabled: true

  port: 7001

  properties:

    port: {{ .Values.plugin.metricReporters.prometheusExporter.port }}

Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 11, 2024
@czy006 czy006 merged commit 0ffdec2 into apache:master Oct 12, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement]: Change the default configuration of values.yaml to improve user experience
3 participants