Page MenuHomePhabricator

Create script to generate library
ClosedPublic

Authored by sashank on Aug 3 2019, 2:27 AM.

Details

Reviewers
max
schrockn
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:6c8085ab85af: Create script to generate library
Summary

Automate generating a dagster library. Uses a template found in bin/assets and a user-provided name to set up a library correctly, since it's easy to miss all the places you have to include the library name.

The generated library includes a simple "helloworld" resource and solid

Test Plan

To use:

python bin/create_library.py --name="sftp"

To test:

cd python_modules/libraries/dagster-sftp
pytest

Diff Detail

Repository
R1 dagster
Branch
create-library-script
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

bin/assets/dagster-library-tmpl/dagster_library-tmpl/version.py
1

Any suggestions on how to correctly set this?

nate added inline comments.
bin/create_library.py
7

nit: copy_directory

looks good to me! but will let @max take a look also before approving

alangenfeld added inline comments.
bin/assets/dagster-library-tmpl/dagster_library-tmpl/__init__.py.tmpl
3–4

hm i wonder how likely these are to be useful vs just need to get deleted. Unclear to me if its better to generate more and delete or generate minimum

bin/assets/dagster-library-tmpl/dagster_library-tmpl/version.py
1

copy paste an existing version.py file

i haven't added any libraries so defer to those who have

schrockn added a subscriber: schrockn.

the resource, solid, and service stuff is too specific. not every library will need that. setup and folder structure is πŸ‘Œ

bin/assets/dagster-library-tmpl/README.md.tmpl
4

libraries are more general than an integration

bin/assets/dagster-library-tmpl/dagster_library-tmpl/resources.py.tmpl
1

imo this is too specific for a generic template

bin/assets/dagster-library-tmpl/dagster_library-tmpl_tests/test_resources.py.tmpl
1

yeah too much

bin/assets/dagster-library-tmpl/dagster_library-tmpl_tests/test_solids.py.tmpl
1

too much

This revision now requires changes to proceed.Aug 11 2019, 9:04 PM
bin/assets/dagster-library-tmpl/README.md.tmpl
4

+1

bin/assets/dagster-library-tmpl/dagster_library-tmpl/__init__.py.tmpl
3–4

yeah, i would skip this and the resources.py / solids.py generation

bin/assets/dagster-library-tmpl/dagster_library-tmpl/version.py
1

could also use the bin/publish.py version machinery

3

this is trickier

bin/assets/dagster-library-tmpl/tox.ini.tmpl
10

i think omit this -- by convention we should force dev-reqs into the dagster file, this is a monorepo

bin/create_library.py
9

consider using .gitignore for this

  • Remove solid, resource, and tests for both
  • Use code from publish.py to set version number
  • Error if library with name already exists
bin/create_library.py
32

nit" let's stick to single quotes

This revision is now accepted and ready to land.Aug 13 2019, 5:50 PM