Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Mar 18, 2019

This is a Draft pull request for initial comments. It's not yet intended to be merged.

The intention is to merge it :)

@potiuk
Copy link
Member Author

potiuk commented Mar 18, 2019

Comment to self (@mik-laj ) - convert the doc to .rst format. @mik-laj - maybe you will help with that ?? I am happy to get your help on that one.

@potiuk potiuk force-pushed the simplified-development-workflow branch 5 times, most recently from 0dcbe8b to 6d155c3 Compare March 18, 2019 17:41
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Non-complete look, just a few spot comments.

@potiuk potiuk force-pushed the simplified-development-workflow branch 2 times, most recently from f4f2aa6 to 8380460 Compare March 18, 2019 21:59
@potiuk potiuk changed the title [AIRFLOW-3611] Simplified development workflow [AIRFLOW-3611] Simplified development workflow [Depends on multi-staging] Mar 18, 2019
@potiuk potiuk force-pushed the simplified-development-workflow branch from 8380460 to 52d9200 Compare March 19, 2019 00:06
@potiuk
Copy link
Member Author

potiuk commented Mar 19, 2019

@mik-laj -> we have .rst Breeze description now.

@potiuk potiuk force-pushed the simplified-development-workflow branch 2 times, most recently from b133d80 to 9263f36 Compare March 19, 2019 01:03
@potiuk
Copy link
Member Author

potiuk commented Mar 19, 2019

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Note to reviewer: Please review only the last commit (use commits section above). This change is based on yet unmerged #4936 change and it contains also commit from that change. It's a github limitation unfortunately.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

@potiuk potiuk force-pushed the simplified-development-workflow branch 5 times, most recently from ae1d111 to e4158cf Compare March 24, 2019 20:36
@potiuk
Copy link
Member Author

potiuk commented Mar 24, 2019

@ashb @Fokko -> I've just added a very nice and easy to setup auto-complete for breee command :). It's becoming a really useful piece of software. Also I am down to just 8 errors in Travis CI.

@potiuk
Copy link
Member Author

potiuk commented Mar 25, 2019

@ashb @Fokko -> as a follow up I also implemented auto-complete for "run-test" method inside the container.

This was by far the single biggest problem I had when running the tests. Having to remember module names and the : in the right place was a major pain. Now it's super convenient to start typing the module name and use auto-complete. It goes down to individual tests.

I also renamed and shortened the run_unit_test.sh method to simple 'run-test' that you can run anywhere (it is in the path). I also added --skip-db-init flag - most of the tests do not require to re-run the database initialisation and if you want to re-run single test, it takes awfully lot of time. So you can now pass --skip-db-init flag to skip resetdb/initdb. You can even autocomplete it ;).

I think with those changes, life of developers trying to use breeze/replicate CI tests will be much, much easier.

I used nosetests for now but we can do very similar thing with pytest when we switch to it.

@potiuk potiuk force-pushed the simplified-development-workflow branch 2 times, most recently from 5b7f8b1 to e2cb22d Compare March 28, 2019 14:43
@potiuk
Copy link
Member Author

potiuk commented Aug 21, 2019

Hello @ashb @mik-laj @andriisoldatenko @dimberman @kaxil @Fokko - I think breeze environment is now ready to be merged. After all the changes merged so far Breeze now becomes a swiss-army-knife of everything-testing for airflow. It builds on top of the CI images and it gives access to all related to testing, static code check (using pre-commits), image management from a single breeze script. I will be talking about it on Monday in NYC and doing workshops on Tuesday so it would be great to merge it before!

@potiuk
Copy link
Member Author

potiuk commented Aug 21, 2019

Comprehensive (and I think very usable) documentation here:
https://github.com/PolideaInternal/airflow/blob/simplified-development-workflow/BREEZE.rst

Besides Breeze will guide users in many places in case of problems/missing prerequisites and the like.

@potiuk potiuk force-pushed the simplified-development-workflow branch 2 times, most recently from a47c3a5 to 61dc768 Compare August 22, 2019 06:09
@dimberman
Copy link
Contributor

Wooo! Ok I'm gonna roll up my sleeves this morning and dive in on this one.

Copy link
Contributor

@dimberman dimberman left a comment

Choose a reason for hiding this comment

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

First look-through this looks good, but I'm gonna hold off an "Approve" until I've had time to actually play with this environment.

@potiuk potiuk force-pushed the simplified-development-workflow branch from 61dc768 to 95c8090 Compare August 22, 2019 17:48
@potiuk
Copy link
Member Author

potiuk commented Aug 22, 2019

I simplified pylint - it is now fully integrated in pre-commit including filtering out files from pylint_todo list - that made it quite a bit simpler for running static checks with breeze (but now it is based on not-yet merged two commits with AIRFLOW-5285)

@potiuk potiuk force-pushed the simplified-development-workflow branch from 95c8090 to e29b639 Compare August 22, 2019 18:46
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

Awesome job @potiuk . A big chunk of work, really appreciate it.

Small nits. I haven't yet tested it but the docs look really solid. Will add more comments once I do test it next week.

@potiuk potiuk force-pushed the simplified-development-workflow branch from e29b639 to 9cc7d0f Compare August 26, 2019 17:10
@potiuk potiuk force-pushed the simplified-development-workflow branch from 9cc7d0f to e79923d Compare August 26, 2019 18:54
@potiuk
Copy link
Member Author

potiuk commented Aug 27, 2019

@kaxil @Fokko @mik-laj @ashb I'd love to merge it today for the workshop today :). No huge problem if it is not merged but it would be great if I can show this from master.

Copy link
Member

@mik-laj mik-laj left a comment

Choose a reason for hiding this comment

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

Good luck!

Copy link
Member

@mik-laj mik-laj left a comment

Choose a reason for hiding this comment

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

Good luck!

@potiuk potiuk merged commit 286aa7a into apache:master Aug 27, 2019
@potiuk
Copy link
Member Author

potiuk commented Aug 27, 2019

Woho! Finally !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-env CI, pre-commit, pylint and other changes that do not change the behavior of the final code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants