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

sanic: 21.3.4 -> 21.9.1, sanic-routing: 0.6.2 -> 0.7.2 #144672

Merged
merged 4 commits into from
Nov 5, 2021
Merged

sanic: 21.3.4 -> 21.9.1, sanic-routing: 0.6.2 -> 0.7.2 #144672

merged 4 commits into from
Nov 5, 2021

Conversation

thiagokokada
Copy link
Contributor

@thiagokokada thiagokokada commented Nov 4, 2021 •
edited
Loading

Motivation for this change

ZHF: #144627

  • Mark pytest-sanic as broken. It doesn't build with newer versions of sanic, since sanic.websocket was removed.
  • Disable some tests that don't work. Most of them are related to the lack of sanic on PATH, so it should be safe.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

thiagokokada
thiagokokada commented Nov 4, 2021
View reviewed changes
Copy link
Contributor Author

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review run on x86_64-linux 1

2 packages marked as broken and skipped:
  • python38Packages.pytest-sanic
  • python39Packages.pytest-sanic
15 packages built:
  • dyndnsc
  • python38Packages.entrance
  • python38Packages.entrance-with-router-features
  • python38Packages.json-logging
  • python38Packages.sanic
  • python38Packages.sanic-auth
  • python38Packages.sanic-routing
  • python38Packages.sanic-testing
  • python39Packages.entrance
  • python39Packages.entrance-with-router-features
  • python39Packages.json-logging
  • python39Packages.sanic
  • python39Packages.sanic-auth
  • python39Packages.sanic-routing
  • python39Packages.sanic-testing

@ofborg ofborg bot requested review from AluisioASG and costrouc November 4, 2021 23:35
thiagokokada
thiagokokada commented Nov 4, 2021
View reviewed changes
pkgs/development/python-modules/pytest-sanic/default.nix
@@ -47,5 +47,6 @@ buildPythonPackage rec {
homepage = "https://github.com/yunstanford/pytest-sanic/";
license = licenses.asl20;
maintainers = with maintainers; [ costrouc ];
broken = true; # 2021-11-04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package is not dependency of any other package now that sanic itself doesn't depend on it.

It is a good candidate for removal.

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

sanic fails the test test_no_exceptions_when_cancel_pending_request on macos 10.15 with:

obj = <Process name='Process-1' parent=68540 initial>
file = <_io.BytesIO object at 0x112e7e400>, protocol = None

    def dump(obj, file, protocol=None):
        '''Replacement for pickle.dump() using ForkingPickler.'''
>       ForkingPickler(file, protocol).dump(obj)
E       AttributeError: Can't pickle local object 'test_no_exceptions_when_cancel_pending_request.<locals>.ping'

which looks awfully similar to the problems in sanic-org/sanic#1774

thiagokokada
thiagokokada commented Nov 5, 2021
View reviewed changes
pkgs/development/python-modules/sanic/default.nix
postPatch = ''
# Loosen dependency requirements.
substituteInPlace setup.py \
--replace '"pytest==5.2.1"' '"pytest"' \
--replace '"pytest==6.2.5"' '"pytest"' \
--replace '"gunicorn==20.0.4"' '"gunicorn"' \
--replace '"pytest-sanic",' "" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While sanic still declares pytest-sanic as dependencies, it doesn't use it for anything as it is clear that the removal also doesn't broke anything.

@thiagokokada
Copy link
Contributor Author

sanic fails the test test_no_exceptions_when_cancel_pending_request on macos 10.15 with:

obj = <Process name='Process-1' parent=68540 initial>
file = <_io.BytesIO object at 0x112e7e400>, protocol = None

    def dump(obj, file, protocol=None):
        '''Replacement for pickle.dump() using ForkingPickler.'''
>       ForkingPickler(file, protocol).dump(obj)
E       AttributeError: Can't pickle local object 'test_no_exceptions_when_cancel_pending_request.<locals>.ping'

which looks awfully similar to the problems in sanic-org/sanic#1774

At least for me many other sanic tests fails on macOS, so I don't think it is worth to trying to make it work.

It already doesn't work currently.

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

Am opening an upstream issue.

@thiagokokada
Copy link
Contributor Author

Am opening an upstream issue.

Could we merge this as-is? We can always fix darwin later (since it is already broken anyway).

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

The other sanic tests that fail on macos, for me at least, are due to attempting to build the python 3.8 and 3.9 versions at the same time, and with the lack of functioning sandboxing on darwin, compete for the same local port.

@thiagokokada
Copy link
Contributor Author

The other sanic tests that fail on macos, for me at least, are due to attempting to build the python 3.8 and 3.9 versions at the same time, and with the lack of functioning sandboxing on darwin, compete for the same local port.

Makes sense. I will just disable the failing tests on macOS them.

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

I think simply disabling that test on darwin will unblock us.

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021 •
edited
Loading

☝️ i.e. that single test. Run the builds one after the other and they're fine for me.

@thiagokokada
Copy link
Contributor Author

@risicle Done.

@risicle
Copy link
Contributor

risicle commented Nov 5, 2021

sanic-org/sanic#2298

@thiagokokada
Copy link
Contributor Author

sanic-org/sanic#2298

Nice. Add issue link as comment in derivation.

@thiagokokada thiagokokada changed the title sanic: 21.3.4 -> 21.9.1, sanic: 21.3.4 -> 21.9.1 sanic: 21.3.4 -> 21.9.1, sanic-routing: 0.6.2 -> 0.7.2 Nov 5, 2021
risicle
risicle approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@risicle risicle left a comment

Choose a reason for hiding this comment

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

nixpkgs-review happy, macos 10.15

@thiagokokada thiagokokada merged commit 5abb42f into NixOS:master Nov 5, 2021
@thiagokokada thiagokokada deleted the zhf-sanic branch November 5, 2021 01:36
SuperSandro2000
SuperSandro2000 reviewed Nov 11, 2021
View reviewed changes
pkgs/development/python-modules/sanic/default.nix
};

patches = [
# Allow later websockets release, https://github.com/sanic-org/sanic/pull/2154
(fetchpatch {
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the unused fetchpatch from inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do any cleanups in this PR: #145022

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

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@risicle risicle risicle approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@AluisioASG AluisioASG Awaiting requested review from AluisioASG

@costrouc costrouc Awaiting requested review from costrouc

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thiagokokada @risicle @SuperSandro2000