-
Notifications
You must be signed in to change notification settings - Fork 835
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
#1297 WIP Update Analytics Helm Chart #1331
Conversation
Mon Jan 13 15:12:32 UTC 2020 impatient try |
Mon Jan 13 15:12:38 UTC 2020 impatient try |
dependencies: | ||
- name: grafana | ||
repository: https://kubernetes-charts.storage.googleapis.com | ||
version: 4.2.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current grafana is major version 6 I think. Are we changing it?
#540
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I was looking at the chart version instead of the application version of Grafana: https://hub.helm.sh/charts/stable/grafana which is currently 4... but I will check that it definitely is the chart version and not the Grafana application version that is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I change version to 6 I get following error:
Error: can't get a valid version for repositories grafana. Try changing the version constraint in Chart.yaml
So I believe it is referring to chart version and not application version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, great can you also confirm the application version, just to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6.5.2
@glindsell Are the .tgz files for grafana and prometheus needed here? |
``` | ||
|
||
``` | ||
helm install seldon-core-analytics . -n seldon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we install this into the seldon-system
namespace by default? Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think we should
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
datasources: | ||
- name: prometheus | ||
type: prometheus | ||
url: http://seldon-core-analytics-prometheus-seldon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So now the prometheus service URL is http://seldon-core-analytics-prometheus-seldon or just http://prometheus-seldon as before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http://seldon-core-analytics-prometheus-seldon/ is now the service url. I made this change in order to know what the url was going to be ahead of time I had to follow the naming convention from the helm install...
command. Is that an issue? Do other applications that use the url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but I think it is better to use that convention and change everything else as needed.
@SachinVarghese |
ns: seldon -> seldon-system
Tue Jan 14 16:19:25 UTC 2020 impatient try |
Tue Jan 14 16:19:29 UTC 2020 impatient try |
Bumps [okhttp](https://github.com/square/okhttp) from 4.2.2 to 4.3.0. - [Release notes](https://github.com/square/okhttp/releases) - [Changelog](https://github.com/square/okhttp/blob/master/CHANGELOG.md) - [Commits](square/okhttp@parent-4.2.2...parent-4.3.0) Signed-off-by: dependabot-preview[bot] <[email protected]>
Bumps [pillow](https://github.com/python-pillow/Pillow) from 6.2.0 to 7.0.0. - [Release notes](https://github.com/python-pillow/Pillow/releases) - [Changelog](https://github.com/python-pillow/Pillow/blob/master/CHANGES.rst) - [Commits](python-pillow/Pillow@6.2.0...7.0.0) Signed-off-by: dependabot-preview[bot] <[email protected]>
New approach is based on getting deyployment names directly from SeldonDeployment objects. This allow to avoid hard-coded hashes in test scripts.
Bumps [okhttp](https://github.com/square/okhttp) from 4.3.0 to 4.3.1. - [Release notes](https://github.com/square/okhttp/releases) - [Changelog](https://github.com/square/okhttp/blob/master/CHANGELOG.md) - [Commits](square/okhttp@parent-4.3.0...parent-4.3.1) Signed-off-by: dependabot-preview[bot] <[email protected]>
Signed-off-by: glindsell <[email protected]>
… into dependencies
Wed Jan 15 13:45:12 UTC 2020 impatient try |
Wed Jan 15 13:45:23 UTC 2020 impatient try |
/lgtm /approve |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glindsell If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
#1297