gino icon indicating copy to clipboard operation
gino copied to clipboard

Sanic extension: typo or bug in listeners?

Open vartagg opened this issue 6 years ago • 1 comments

Sanic extension: typo or bug in listeners?

  • GINO version: 0.8.5
  • Python version: 3.7.4
  • asyncpg version: 0.20.0
  • aiocontextvars version: 0.2.2
  • PostgreSQL version: 11.5
  • sanic version: 19.9.0

Description

I have the issue with my server in Sanic and Gino with Sanic extension. The server is running in Kubernetes, and looks like the K8s /health probe on server start is executing before Gino was actually initialized and Gino engine is not initialized exception raised. I read the code of Gino in gino/ext/sanic.py and found that the listeners use signals after_server_start and before_server_stop. I assume, that the first signal should be before_server_start. Also, the decorated functions use the names differ than arguments passed to decorators.

What I Did

# app/__init__.py
db.init_app(app)

@app.route('/health', methods=['GET', 'HEAD'])
async def health(request: Request):
    if request.method == 'GET':
        return text('Ok', 200)
    return text(None, 200)

The code snippet from gino/ext/sanic.py

        @app.listener('after_server_start')
        async def before_server_start(_, loop):
            if app.config.get('DB_DSN'):
                dsn = app.config.DB_DSN
            else:
                dsn = URL(
                    drivername=app.config.setdefault('DB_DRIVER', 'asyncpg'),
                    host=app.config.setdefault('DB_HOST', 'localhost'),
                    port=app.config.setdefault('DB_PORT', 5432),
                    username=app.config.setdefault('DB_USER', 'postgres'),
                    password=app.config.setdefault('DB_PASSWORD', ''),
                    database=app.config.setdefault('DB_DATABASE', 'postgres'),
                )

            await self.set_bind(
                dsn,
                echo=app.config.setdefault('DB_ECHO', False),
                min_size=app.config.setdefault('DB_POOL_MIN_SIZE', 5),
                max_size=app.config.setdefault('DB_POOL_MAX_SIZE', 10),
                ssl=app.config.setdefault('DB_SSL'),
                loop=loop,
                **app.config.setdefault('DB_KWARGS', dict()),
            )

        @app.listener('before_server_stop')
        async def after_server_stop(_, loop):
            await self.pop_bind().close()

^ first listener uses argument after_server_start, but decorated function name is before_server_start. Same for second listener: before_server_stop argument and method is after_server_stop.

vartagg avatar Dec 13 '19 05:12 vartagg

The change was done in #520, with reasons. The method name changes were missed there.

wwwjfy avatar Dec 21 '19 17:12 wwwjfy