1795 rename async module #1796

diegoholiveira · 2018-05-26T13:43:19Z

benoitc · 2018-05-26T19:53:18Z

renaming it _async would mean that module is private where some can import its objects to create other async workers. I would rather rename it as async_base or base_async whatever.

benoitc

see comment above

diegoholiveira · 2018-05-26T20:14:18Z

changes requested done ;)

benoitc

Thanks! looks good for me, just wondering if the change should be major. Thoughts ? cc @berkerpeksag @tilgovi

benoitc · 2018-06-01T08:36:05Z

@tilgovi @berkerpeksag thoughts? I am thinking we should create a pre-20 branch that include that change and the removal of python 2. Thoughts?

berkerpeksag

Thanks! looks good for me, just wondering if the change should be major.

I don't think this is a major change since the chance of having worker subclasses is low and we are not the only project to make this change.

I'd vote +1 for adding this to the next 19.x release.

berkerpeksag · 2018-06-03T13:49:54Z

requirements_test.txt

@@ -1,3 +1,3 @@
 coverage>=4.0,<4.4  # TODO: https://github.com/benoitc/gunicorn/issues/1548
-pytest==3.0.5
-pytest-cov==2.4.0
+pytest


Please leave unrelated changes to another PR.

berkerpeksag · 2018-06-03T13:50:16Z

docs/source/news.rst

+19.9.0 / 2018/05/26
+===================
+
+- the internal module `gunicorn.workers.async` was renamed to `gunicorn.workers.base_async`


berkerpeksag · 2018-06-03T13:50:54Z

docs/source/news.rst

+19.9.0 / 2018/05/26
+===================
+
+- the internal module `gunicorn.workers.async` was renamed to `gunicorn.workers.base_async`


Style nit: Please use double backtics.

berkerpeksag · 2018-06-03T13:51:46Z

docs/source/news.rst

@@ -2,6 +2,14 @@
 Changelog
 =========

+19.9.0 / 2018/05/26


I would change 2018/05/26 with "not released".

berkerpeksag · 2018-06-03T13:52:42Z

.gitignore

@@ -15,3 +15,5 @@ examples/frameworks/pylonstest/pylonstest.egg-info/
 MANIFEST
 nohup.out
 setuptools-*
+.cache


.cache was renamed to .pytest_cache and we already added it to .gitignore.

berkerpeksag · 2018-06-03T13:53:55Z

.gitignore

@@ -15,3 +15,5 @@ examples/frameworks/pylonstest/pylonstest.egg-info/
 MANIFEST
 nohup.out
 setuptools-*
+.cache
+.eggs


This looks like a good candidate for having a global .gitignore file.

diegoholiveira · 2018-06-03T14:42:25Z

@berkerpeksag done

berkerpeksag

Looks great, thank you!

mozillazg · 2018-06-08T15:38:05Z

gunicorn/workers/__init__.py

@@ -17,6 +17,6 @@
 }


-if sys.version_info >= (3, 3):
+if sys.version_info >= (3, 4):
    # gaiohttp worker can be used with Python 3.3+ only.


3.3+ -> 3.4+?

I did this change to match this: https://github.com/benoitc/gunicorn/blob/master/gunicorn/workers/gaiohttp.py#L10

I mean the comment # gaiohttp worker can be used with Python 3.3+ only. should be updated to # gaiohttp worker can be used with Python 3.4+ only. to match your change.

…5-rename-async-module

tilgovi · 2018-06-17T20:23:17Z

Merged! Thank you all for the PR and for the reviews. I'll try to make a release by next weekend so that hopefully Gunicorn is ready when 3.7 is released.

berkerpeksag · 2018-06-18T04:07:43Z

Can we please use the "squash and merge" option when there are intermediate commits in a PR? Commits like 07dc716 and 66ec021 shouldn't be merged into the master branch.

We don't have feature branches and develop features the way the Linux kernel team does (long lived branches, no intermediate commits etc.), so I think we can even disable the "create a merge commit" option.

tilgovi · 2018-06-18T16:25:15Z

I agree that we should avoid having fixup commits like that. They're not useful.

I don't "squash and merge" because sometimes the commits matter. I'll try to be more careful, though, and review the commits.

Maybe we can agree on a review process. When you give a ✔️ a usually trust it, but maybe we should ask for rebase / squash by the authors before we approve. Small risk of making people frustrated, but lower risk of accidentally squashing useful history or merging useless history.

benoitc · 2018-06-20T07:57:45Z

@berkerpeksag well we should probably have long lived branch. So we can break from now the code and work on next major release and start new iterations from it :) For example, I wouldn't have merged it before bumping our own version. That change will break anyone using a custom worker based on the async base. So that's a good call for such branches :)

diegoholiveira added 2 commits May 26, 2018 10:22

Move the module async to _async

0a88d19

Fix gaiohttp worker

43e31c3

benoitc requested changes May 26, 2018

View reviewed changes

Change _async to base_async

5a82e7c

benoitc reviewed May 27, 2018

View reviewed changes

benoitc approved these changes May 27, 2018

View reviewed changes

benoitc requested a review from berkerpeksag May 27, 2018 11:20

berkerpeksag requested changes Jun 3, 2018

View reviewed changes

Fix style issues and revert some unrelated changes

72dde43

diegoholiveira mentioned this pull request Jun 3, 2018

Upgrade pytest and pytest-cov plugin #1799

Merged

markup tweaks

07dc716

berkerpeksag approved these changes Jun 3, 2018

View reviewed changes

benoitc self-requested a review June 6, 2018 13:44

mozillazg reviewed Jun 8, 2018

View reviewed changes

diegoholiveira added 3 commits June 9, 2018 10:36

Fix comment about python version

d338fe1

Merge branch 'master' of https://github.com/benoitc/gunicorn into 179…

e81a946

…5-rename-async-module

Merge branch 'master' into 1795-rename-async-module

557b919

tilgovi merged commit bd833e0 into benoitc:master Jun 17, 2018

diegoholiveira deleted the 1795-rename-async-module branch June 28, 2018 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1795 rename async module #1796

1795 rename async module #1796

diegoholiveira commented May 26, 2018

benoitc commented May 26, 2018

benoitc left a comment

diegoholiveira commented May 26, 2018

benoitc left a comment •

edited

Loading

benoitc commented Jun 1, 2018

berkerpeksag left a comment

berkerpeksag Jun 3, 2018

berkerpeksag Jun 3, 2018

berkerpeksag Jun 3, 2018

berkerpeksag Jun 3, 2018

berkerpeksag Jun 3, 2018

berkerpeksag Jun 3, 2018

diegoholiveira commented Jun 3, 2018

berkerpeksag left a comment

mozillazg Jun 8, 2018

diegoholiveira Jun 8, 2018

mozillazg Jun 9, 2018 •

edited

Loading

tilgovi commented Jun 17, 2018

berkerpeksag commented Jun 18, 2018

tilgovi commented Jun 18, 2018

benoitc commented Jun 20, 2018

1795 rename async module #1796

1795 rename async module #1796

Conversation

diegoholiveira commented May 26, 2018

benoitc commented May 26, 2018

benoitc left a comment

Choose a reason for hiding this comment

diegoholiveira commented May 26, 2018

benoitc left a comment • edited Loading

Choose a reason for hiding this comment

benoitc commented Jun 1, 2018

berkerpeksag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoholiveira commented Jun 3, 2018

berkerpeksag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mozillazg Jun 9, 2018 • edited Loading

Choose a reason for hiding this comment

tilgovi commented Jun 17, 2018

berkerpeksag commented Jun 18, 2018

tilgovi commented Jun 18, 2018

benoitc commented Jun 20, 2018

benoitc left a comment •

edited

Loading

mozillazg Jun 9, 2018 •

edited

Loading