Skip to content

Conversation

@TomAugspurger
Copy link
Member

I believe these docs were out of date.

@jacobtomlinson
Copy link
Member

Thanks @TomAugspurger.

We use Frigate here for autogenerating the README. The main benefit is that it keeps the table of configuration options up to date. Would you mind updating the template and regenerating the REAMDE?

https://github.com/dask/helm-chart#generating-the-readme

(I made some changes to Frigate this week which will make it super useful for daskhub as it will pull all the possible configuration options from the dependency charts too)

@jacobtomlinson
Copy link
Member

This whole process of using a template and generating the README from it is a little annoying. Frigate also has a sphinx plugin so we may just want to move the docs to RTD instead of the README to make the whole experience more pleasant.

dask/README.md Outdated
| `scheduler.image.pullSecrets` | Container image [pull secrets](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/). | `null` |
| `scheduler.replicas` | Number of schedulers (should always be 1). | `1` |
| `scheduler.serviceType` | Scheduler service type. set to `loadbalancer` to expose outside of your cluster. | `"ClusterIP"` |
| `scheduler.serviceType` | Scheduler service type. set to `LoadBalancer` to expose outside of your cluster. | `"ClusterIP"` |
Copy link
Member

Choose a reason for hiding this comment

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

This may be a bug in Frigate because the comment in the values.yaml this is generated from looks correct.

https://github.com/dask/helm-chart/blob/master/dask/values.yaml#L14

Copy link
Member Author

Choose a reason for hiding this comment

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

If I had to guess it's https://github.com/rapidsai/frigate/blob/874493d8fbfd8c7fe94f13a709bcf7bf92cc680b/frigate/gen.py#L184.

I think it'd be best if frigate didn't try to be clever here. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think dropping the capitalise call would be best.

TomAugspurger added a commit to TomAugspurger/frigate that referenced this pull request Aug 25, 2020
As noted in dask/helm-chart#92 (comment),
in some comments capitalization is important so frigate shouldn't change
it.
jacobtomlinson pushed a commit to rapidsai/frigate that referenced this pull request Aug 25, 2020
As noted in dask/helm-chart#92 (comment),
in some comments capitalization is important so frigate shouldn't change
it.
@consideRatio consideRatio added the chart/dask Related to the dask chart label Nov 6, 2020
Base automatically changed from master to main February 11, 2021 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chart/dask Related to the dask chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants