Page MenuHomePhabricator

Improve API docs
ClosedPublic

Authored by max on Thu, Oct 31, 9:22 PM.

Details

Reviewers
alangenfeld
prha
schrockn
Group Reviewers
Restricted Project
Commits
R1:c5f4b5d45c03: Improve API docs
Summary

Better doc coverage of the public API surface

Test Plan

Unit

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

max created this revision.Thu, Oct 31, 9:22 PM
max updated this revision to Diff 6132.Thu, Oct 31, 9:33 PM

Revert notebook

prha added a comment.Thu, Oct 31, 9:50 PM

yesss

this looks great!

docs/sections/api/apidocs/types.rst
19

Holy smokes... I love these type example sections.

python_modules/dagster-graphql/dagster_graphql/schema/runs.py
457

Don't we standardize to camelCase for graphql?

alangenfeld accepted this revision.Thu, Oct 31, 9:50 PM

this is another unreal leap in quality - great work

werenotworthy

some nits inline, proceed at your discretion

docs/sections/api/apidocs/execution.rst
43–44

whats this combo do?

docs/sections/api/apidocs/solids.rst
10

Simple

this adjective feels off here to me, would prefer Core solids, Compute solids, no title. To clarify I feel like simple doesn't communicate anything

52

maybe worth calling out that Failure is expected to be raised not yielded

js_modules/dagit/src/RunMetadataProvider.tsx
277

this seems wrong - whats going on here?

python_modules/dagster/dagster/core/definitions/decorators.py
172

what does this mean? can we take care of it before landing?

230

resolve or gh issue link

python_modules/dagster/dagster/core/definitions/resource.py
11–32

and to clean up after execution resolve

Should this block discuss that the function can be a yield-once context manager style function to allow clean-up on teardown?

This revision is now accepted and ready to land.Thu, Oct 31, 9:50 PM
max marked 8 inline comments as done.Thu, Oct 31, 10:14 PM
max added inline comments.
docs/sections/api/apidocs/execution.rst
43–44
docs/sections/api/apidocs/solids.rst
10

i did go looking for synonyms for noncomposite

52

yep

js_modules/dagit/src/RunMetadataProvider.tsx
277

fixing

python_modules/dagster-graphql/dagster_graphql/schema/runs.py
457

the python code should be snake_case, the camelcase conversion should occur when we go to js

python_modules/dagster/dagster/core/definitions/decorators.py
172

yep

230

yup

python_modules/dagster/dagster/core/definitions/resource.py
11–32

yes, definitely

max updated this revision to Diff 6136.Thu, Oct 31, 10:15 PM
max marked 7 inline comments as done.

Nits

max updated this revision to Diff 6145.Thu, Oct 31, 10:34 PM

Remove expectations config

This revision was landed with ongoing or failed builds.Thu, Oct 31, 10:47 PM
This revision was automatically updated to reflect the committed changes.
Harbormaster failed remote builds in B4927: Diff 6136!