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.

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

fix signal issue (port of #63) #66

Closed
wants to merge 2 commits into from

Conversation

indam23
Copy link

@indam23 indam23 commented Jul 1, 2022

No description provided.

@indam23 indam23 changed the title port fix from https://github.com/yunstanford/pytest-sanic/pull/63 fix signal issue (port of #63) Jul 1, 2022
@indam23 indam23 mentioned this pull request Jul 1, 2022
@ancalita
Copy link

ancalita commented Jul 1, 2022

@melindaloubser1 Those extra 2 lines of code seem to cause some Sanic errors in test_nlg among others, I recommend removing them and keeping only the 1 line :

await self.app._startup()

pytest_sanic/utils.py Outdated Show resolved Hide resolved
@indam23
Copy link
Author

indam23 commented Jul 4, 2022

I've made one update according to this comment, testing it now

@indam23
Copy link
Author

indam23 commented Jul 4, 2022

With the loop specifies it works with the additions

@notzippy
Copy link

There is one more statement that needs to be called now with Sanic 22.6 branch app.prepare I called it like this

     server_settings.pop("main_start", None)
    server_settings.pop("main_stop", None)

   # Added the prepare call with setting the state flage `self.app.asgi `
    if not self.app.state.server_info:
        self.app.prepare()
        # Alternately asgi_client = self.app.asgi_client
        self.app.asgi = True

    # We need to invoke the server directly
    await self.app._startup()
    await self.app._server_event("init", "before", loop=self.loop)

@indam23
Copy link
Author

indam23 commented Aug 15, 2022

@notzippy If you want to take this over feel free, for our case we kept using our fork.

@notzippy
Copy link

@notzippy If you want to take this over feel free, for our case we kept using our fork.

Ive kinda derived a custom version for my own from this repo, so I was just contributing back :-)

@yunstanford
Copy link
Owner

yunstanford commented Aug 15, 2022

Hey guys @notzippy @melindaloubser1 , sorry about the late replies and inconvenience, kinda of busy these days, not able to keep maintaining the lib.

I'm happy to add you guys as maintainers (github and pypi) as well

@indam23
Copy link
Author

indam23 commented Aug 16, 2022

all good, not needed in my case at least. I'll close this for now then.

@indam23 indam23 closed this Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants