Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactoring utils to be more sane #1219

Merged
merged 1 commit into from
Mar 29, 2016
Merged

Refactoring utils to be more sane #1219

merged 1 commit into from
Mar 29, 2016

Conversation

artwr
Copy link
Contributor

@artwr artwr commented Mar 26, 2016

utils had become a little too complex. Other projects like Django
or IPython have a more structured and, I would argue, clearer way
to organize utils. I try to reproduce this here.

Ideally we want a utils folder with submodules that are grouped
thematically. I rebased off of master and fixed references across
the repository. I also introduced a PendingDeprecationWarning for
calling apply_defaults from airflow.utils directly and redirect
people to the right place. I also moved exceptions to a top level
file.

Supersedes: #1098

@landscape-bot
Copy link

Code Health
Repository health increased by 0.74% when pulling 0b43aad on arthurw_utils_refactor_take2 into 7a632e2 on master.

@artwr artwr mentioned this pull request Mar 26, 2016
Closed
@artwr
Copy link
Contributor Author

artwr commented Mar 26, 2016

@mistercrunch as discussed.
@aoen since you were nice enough to review the first one.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 0b43aad to 188d627 Compare March 26, 2016 05:03
@landscape-bot
Copy link

Code Health
Repository health increased by 0.87% when pulling 188d627 on arthurw_utils_refactor_take2 into 7a632e2 on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 188d627 to e118d63 Compare March 26, 2016 05:31
@landscape-bot
Copy link

Code Health
Repository health increased by 0.87% when pulling e118d63 on arthurw_utils_refactor_take2 into 7a632e2 on master.

@bolkedebruin
Copy link
Contributor

From a first glance looks good! Would you mind updating your commit message per these guidelines http://chris.beams.io/posts/git-commit/ . It helps creating changelogs and release notes in the future.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from e118d63 to 5a04c38 Compare March 26, 2016 08:08
@artwr
Copy link
Contributor Author

artwr commented Mar 26, 2016

Thanks. I tried to make the title more explicit. I think there is still a small issue with the email tests.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.87% when pulling 5a04c38 on arthurw_utils_refactor_take2 into 7a632e2 on master.

@bolkedebruin
Copy link
Contributor

Would you mind rebasing? Let's see if we can merge then. Thanks for the commit message, it looks so much nicer ;-)

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 5a04c38 to 9c0fba9 Compare March 28, 2016 13:31
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling 9c0fba9 on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 9c0fba9 to 13efa9e Compare March 28, 2016 13:35
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling 13efa9e on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 13efa9e to 79568b9 Compare March 28, 2016 17:30
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling 79568b9 on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 79568b9 to 055648d Compare March 28, 2016 17:59
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling 055648d on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 055648d to b9be025 Compare March 28, 2016 18:26
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling b9be025 on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from b9be025 to 11576bc Compare March 28, 2016 18:39
@landscape-bot
Copy link

Code Health
Repository health increased by 0.84% when pulling 11576bc on arthurw_utils_refactor_take2 into 1db892b on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 66.149% when pulling 11576bcb52b2af63b57133c32a1d017721efb34c on arthurw_utils_refactor_take2 into 1db892b on master.

@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 11576bc to 7db5825 Compare March 28, 2016 19:39
@landscape-bot
Copy link

Code Health
Repository health increased by 0.79% when pulling 7db5825 on arthurw_utils_refactor_take2 into c14d2ea on master.

@artwr
Refactoring utils into smaller submodules
773f52f 
utils.py had become a little too complex. Other projects like
Django or IPython have a more structured and, I would argue,
clearer way to organize utils. I try to reproduce this here.

Ideally we want a utils folder with submodules that are grouped
thematically. I rebased off of master and fixed references across
the repository. I also introduced a PendingDeprecationWarning for
calling apply_defaults from `airflow.utils` directly and redirect
people to the right place. I also moved exceptions to a top level
file.
@artwr artwr force-pushed the arthurw_utils_refactor_take2 branch from 7db5825 to 773f52f Compare March 28, 2016 20:24
@landscape-bot
Copy link

Code Health
Repository health increased by 0.94% when pulling 773f52f on arthurw_utils_refactor_take2 into c14d2ea on master.

aoen
aoen reviewed Mar 28, 2016
View reviewed changes
airflow/jobs.py
from airflow.utils import AirflowException, State, LoggingMixin
from airflow.exceptions import AirflowException
from airflow.utils.state import State
from airflow.utils.db import provide_session, pessimistic_connection_handling
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit(feel free to ignore): ideally alphasort the depdendencies in all lines like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make sure I do in a following PR. At this point, I would rather merge while it works, because rebasing on that scale is a little bit of a PITA.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 67.186% when pulling 773f52f on arthurw_utils_refactor_take2 into c14d2ea on master.

@artwr
Copy link
Contributor Author

artwr commented Mar 29, 2016

Alright, merging after 👍.

@artwr artwr merged commit 60e22ed into master Mar 29, 2016
@jlowin jlowin mentioned this pull request Apr 3, 2016
Closed
@artwr artwr deleted the arthurw_utils_refactor_take2 branch April 5, 2016 18:39
@dimberman dimberman mentioned this pull request Mar 29, 2020
Closed
mobuchowski pushed a commit to mobuchowski/airflow that referenced this pull request Jan 4, 2022
@wslulciuc
Unify common deps gradle (apache#1219)
dd0edba 
* Add global dependencies to build.gradle

Signed-off-by: wslulciuc <willy@datakin.com>

Use vars for deps in build.gradle

Signed-off-by: wslulciuc <willy@datakin.com>

* Add ext.postgresqlVersion

Signed-off-by: wslulciuc <willy@datakin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@artwr @landscape-bot @bolkedebruin @coveralls @aoen