Page MenuHomePhabricator

Revised the Parametrizing Solid with Config in Docs Basics of Solids
ClosedPublic

Authored by yichendai on Thu, Oct 8, 3:34 AM.

Details

Summary
  1. Created a new config.py for parametrizing-solids-with-config. Wrote a new config.py and its test: test_config.py.
  2. The image is modified manually. I'll update all the screenshots after learning how to use generate_screenshots.test.js

Latest Preview by 10/19 PM:

Test Plan

test_config.py

Diff Detail

Repository
R1 dagster
Branch
configSolid
Lint
Lint OK
Unit
No Unit Test Coverage

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.Thu, Oct 8, 6:23 AM
Harbormaster failed remote builds in B19305: Diff 23457!
Harbormaster returned this revision to the author for changes because remote builds failed.Thu, Oct 8, 5:35 PM
Harbormaster failed remote builds in B19322: Diff 23482!
yichendai retitled this revision from Revised the Prametrizing Solid with Config in Docs Basics of Solids to Revised the Parametrizing Solid with Config in Docs Basics of Solids.Thu, Oct 8, 7:28 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Thu, Oct 8, 7:46 PM
Harbormaster failed remote builds in B19331: Diff 23492!

Update: Created a new test for config.py

yichendai removed reviewers: max, yuhan, sashank.
Harbormaster returned this revision to the author for changes because remote builds failed.Tue, Oct 13, 7:04 AM
Harbormaster failed remote builds in B19423: Diff 23601!

Update: Fix import error in test_config.py and merge conflicts

yichendai retitled this revision from Revised the Parametrizing Solid with Config in Docs Basics of Solids to [Part1] Revised the Parametrizing Solid with Config in Docs Basics of Solids.Wed, Oct 14, 5:51 PM
yichendai edited the summary of this revision. (Show Details)
yichendai edited the test plan for this revision. (Show Details)
yichendai added reviewers: yuhan, max, sashank.
yichendai edited the summary of this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Wed, Oct 14, 5:56 PM
Harbormaster failed remote builds in B19508: Diff 23701!
yichendai edited the summary of this revision. (Show Details)
yichendai edited the summary of this revision. (Show Details)
yichendai edited the summary of this revision. (Show Details)

Update: black fix formatting; update config_env.yaml

Harbormaster returned this revision to the author for changes because remote builds failed.Wed, Oct 14, 7:38 PM
Harbormaster failed remote builds in B19514: Diff 23707!

Update: black reformat config.py

Harbormaster returned this revision to the author for changes because remote builds failed.Wed, Oct 14, 9:45 PM
Harbormaster failed remote builds in B19527: Diff 23722!
yichendai retitled this revision from [Part1] Revised the Parametrizing Solid with Config in Docs Basics of Solids to Revised the Parametrizing Solid with Config in Docs Basics of Solids.Thu, Oct 15, 5:31 PM
yichendai edited the summary of this revision. (Show Details)
yichendai edited the summary of this revision. (Show Details)
yichendai edited the summary of this revision. (Show Details)

can you screenshot the final view of the page for reviewing?

Here're three screenshots for preview of the page.

would be good to show how users can specify the solid config value in [1] and [2] too as you did in the test

examples/docs_snippets/docs_snippets/intro_tutorial/basics/e02_solids/config.py
63

[1] supply run_config via python api

examples/docs_snippets/docs_snippets/intro_tutorial/basics/e02_solids/config_env.yaml
6 ↗(On Diff #23764)

[2] use CLI with the yaml file

yichendai added inline comments.
docs/next/src/pages/tutorial/basics_solids.mdx
249

appears

yichendai edited the summary of this revision. (Show Details)

Update: Add how to specify config by python api and yaml file

good job! this is looking a lot better

docs/next/src/pages/tutorial/basics_solids.mdx
222–224

could also emphasize the line with the solid config

examples/docs_snippets/docs_snippets/intro_tutorial/basics/e02_solids/config.py
37–46

after reading through the screenshot, i don't think these log information make much sense. when reverse=True, sorted_cereals[0] will be the most and sorted_cereals[-1] will be the least.

i'd suggest maybe come up with a better example semantically, for example (just throwing ideas), only printing:

context.log.info(
    "{x} caloric cereal: {least_caloric}".format(
        x='most' if reverse else 'least',
        least_caloric=sorted_cereals[0]["name"]
    )
)

Update: modified context.log.info and set reverse to True in config.py and its yaml

This is great! Some minor feedback. Big improvement

docs/next/src/pages/tutorial/basics_solids.mdx
202–203

We shouldn't wrap the description in a tuple

209

we may want to note here that they can optionally be *only* the type if you do not require changes to other defaults.

In fact we may want to consider starting with:

@solid(config_schema={"reverse" : bool})

Because it is less verbose, and then explain how we can opt into descriptions and default values and required

This revision now requires changes to proceed.Mon, Oct 19, 7:26 PM

Update: Start with the least verbose config example

yichendai marked 2 inline comments as done.EditedTue, Oct 20, 12:49 AM

The config.py contains the least verbose example.

Created another config_more_details.py to hold the optional config.

yichendai edited the summary of this revision. (Show Details)
yichendai edited the summary of this revision. (Show Details)

awesome! looking forward to seeing this pushed

docs/next/src/pages/tutorial/basics_solids.mdx
227–228

It might be worth nothing here that bool is automatically coerced to Field(Bool) and is precisely equivalent

This revision is now accepted and ready to land.Tue, Oct 20, 3:24 PM
yichendai edited the summary of this revision. (Show Details)

Update last fix