Page MenuHomeElementl

k8s deployment docs
ClosedPublic

Authored by catherinewu on Dec 10 2020, 10:30 PM.

Details

Reviewers
nate
Group Reviewers
Restricted Project
Commits
R1:f73c4d537837: k8s deployment docs
Summary

updated k8s deployment docs. would love to hear thoughts / feedback.

todo: the link to daemon docs + some daemon descriptions still need to be filled in. given the change from enabled -> enums, some of the commands will need to be updated

Test Plan

went through both deployment guides (and fixed the issues that i ran into). if this looks good, next step is to add tests for the deployment guide.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 5 2021, 1:51 AM

is it possible to link to a release version of the values.yaml file? otherwise, there will be potential for skew between latest published helm chart and the values.yaml on master

nate added inline comments.
docs/next/src/pages/deploying/k8s.mdx
20

should this be values.yaml (code-formatted) instead of italics?

also per discussion this morning, maybe we can link to the specific version based on the docs version? @sashank probably has thoughts on how best to do this

24

these should all be relative links /deploying/k8s#default-deployment - can remove https://docs.dagster.io prefix throughout this file

also nit: can the whole thing including the arrow be the link? so this line would be as in the suggested edit

27

nit: can these two references be links instead of just italics?

51

nit: PostgreSQL

52

is this too much detail? Maybe 3 and 4 could be combined as:

The Daemon checks (every <100ms) for all runs that are ready to be executed, finds the Pipeline Run from (2), and spins up a Run Worker, which is an ephemeral K8s Job (with image company_repository/baz:6).

decision to include the timing depends on how much we expect the reader to be concerned about run start latency

61

can you add a UI component to format these k8s components as a table for readability?

111
115

should this section go above the "Default Deployment" section? feels like it'd be nice to get it running first, then understand the components I just deployed

127

we should ensure these links are pinned to release versions cc @sashank

158

as earlier, relative link here

166

can we make these the defaults for 0.10.0 so that you don't need CLI --set flags here?

197

can we add a Dagit screenshot for this?

221

nice to have only: can we make "[same as default]" a styled label w/ a component + some CSS? maybe white text on blue background or something - can imagine we might want more generalized labels like "NOTE:", "Warning!" etc. for different contexts

cc @sashank

235

would be great to have this as a table also for readability

298

Can we link to GCS docs here or something? I think quite a few of our users are on GKE + GCS

301

accidentally a word here

386

nit: make this multi-line w/ \ after each argument for readability?

392

nit: make this multi-line w/ \ after each argument for readability?

395

nit: no :

407

nit: no :

529

database logins*

This is great, thanks for all the hard work to put this together! Added a first round of comments, will do another review pass when you're ready for it

docs/next/src/pages/deploying/k8s.mdx
20

I prefer the aesthetics of italics for file names and only code in code-format, but will change to code-format for consistency.

imo link to the specific version based on the docs version should be a follow on task unless @sashank you know of a clever way to do this?

24

done

i originally did not want to do "whole thing including the arrow be the link" since the aesthetics looked off to me. removing the semibold formatting per our convo makes it a lot easier on the eyes imo. done

27

done

51

nice. fixed this elsewhere too

52

consolidated. users have expressed concern about the latency of the daemon

61

nice idea, done

111

done

115

not sure, but i moved the quickstart to be the first section per your suggestion

158

done

166

i'd prefer to keep the full command here until the default in helm settles down (i don't yet know which fields will become enums, what the default values are going to be, etc). Also this is a bit safer till these guides are under test (protects against people changing defaults w/o changing guide)

197

is there something in particular that we're interested in capturing? can i link out to a generic how to launch executions in dagit guide?

221

given our new docs site tba, will leave it up to sashank?

235

done

298

done

301

removed

386

done

392

done

395

done

407

done

529

done

catherinewu retitled this revision from [rfc] k8s deployment docs to k8s deployment docs.Jan 11 2021, 6:30 PM
docs/next/src/pages/deploying/k8s.mdx
125

run a*

156

nit: shoudl this be Walkthrough? I'm not sure which is the more correct form actually

171

not critical, but is there an easy way to style this? would help legibility if the header could be a shade of gray and then the rows alternating colors white/light gray or something

173

for consistency with 2 rows down—maybe remove "K8s" from all of these?

177

suggesting for consistency with other rows

192

something like this? feel free to reword

209
217

should we spell out "advanced" here? "Adv" looks a little funny to my eyes

362

ha, don't miss this TODO!

489

should this be code-styled?

catherinewu marked 8 inline comments as done.

address comments

This looks really good to me now! Thanks for getting this done!!

This revision is now accepted and ready to land.Jan 13 2021, 12:20 AM
This revision was automatically updated to reflect the committed changes.