schireson / pytest-mock-resources

Pytest Fixtures that let you actually test against external resource (Postgres, Mongo, Redshift...) dependent code.
https://pytest-mock-resources.readthedocs.io/en/latest/quickstart.html
MIT License
179 stars 19 forks source link

Add support for MySQL 8 #131

Closed cobed95 closed 2 years ago

cobed95 commented 2 years ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

It seems that MySQL 8 (specifically 8.0.23) requires setting only MYSQL_ROOT_PASSWORD when using 'root' user.

2021-12-03 09:32:36+00:00 [Note] [Entrypoint]: Entrypoint script for MySQL Server 8.0.23-1debian10 started.
2021-12-03 09:32:36+00:00 [Note] [Entrypoint]: Switching to dedicated user 'mysql'
2021-12-03 09:32:36+00:00 [Note] [Entrypoint]: Entrypoint script for MySQL Server 8.0.23-1debian10 started.
2021-12-03 09:32:36+00:00 [ERROR] [Entrypoint]: MYSQL_USER="root", MYSQL_USER and MYSQL_PASSWORD are for configuring a regular user and cannot be used for the root user
    Remove MYSQL_USER="root" and use one of the following to control the root user password:
    - MYSQL_ROOT_PASSWORD
    - MYSQL_ALLOW_EMPTY_PASSWORD
    - MYSQL_RANDOM_ROOT_PASSWORD

However, private fixture _mysql_container is giving MYSQL_USER and MYSQL_ROOT_PASSWORD together, and because 'root' is the default value of MysqlConfig's username field, default config can never run correctly.

One might consider overriding the username field, but that requires setting MYSQL_PASSWORD environment variable to correctly create the custom user.

2021-12-03 09:31:33+00:00 [Warn] [Entrypoint]: MYSQL_USER specified, but missing MYSQL_PASSWORD; MYSQL_USER will not be created

I have tried overriding the _mysql_container fixture to provide additional environment variable with name MYSQL_PASSWORD, but the created user seems to be missing some privileges internally required by pytest-mock-resources.

data = b"\xff\x14\x04#42000Access denied for user 'test'@'%' to database 'pytest_mock_resource_db_1'"

    def raise_mysql_exception(data):
        errno = struct.unpack("<h", data[1:3])[0]
        errval = data[9:].decode("utf-8", "replace")
        errorclass = error_map.get(errno)
        if errorclass is None:
            errorclass = InternalError if errno < 1000 else OperationalError
>       raise errorclass(errno, errval)
E       sqlalchemy.exc.OperationalError: (pymysql.err.OperationalError) (1044, "Access denied for user 'test'@'%' to database 'pytest_mock_resource_db_1'")
E       [SQL: CREATE DATABASE pytest_mock_resource_db_1]
E       (Background on this error at: https://sqlalche.me/e/14/e3q8)

../../../../venv/lib/python3.7/site-packages/pymysql/err.py:143: OperationalError

Describe the solution you'd like A clear and concise description of what you want to happen.

A proper support for different versions of MySQL, or additional fields in MysqlConfig to allow for a more graceful overriding.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

I'm currently manually overriding the private _mysql_container_fixture as a temporary workaround.

@pytest.fixture(scope="session")
def _mysql_container(pmr_mysql_config):
    result = get_container(
        pmr_mysql_config,
        {3306: pmr_mysql_config.port},
        {
            "MYSQL_DATABASE": pmr_mysql_config.root_database,
            "MYSQL_ROOT_PASSWORD": pmr_mysql_config.password,
        },
        check_mysql_fn,
    )

    yield next(iter(result))

Additional context Add any other context or screenshots about the feature request here.

DanCardin commented 2 years ago

Yea I ran into this the other day implementing https://github.com/schireson/pytest-mock-resources/pull/129/files#diff-4c6c8cd8e1ba97c69fc5b49df46ad19168ea302a8d3e9e2f95ea53ea8109b56aL99, which is otherwise unrelated. Since we default to such an old mysql container, i wonder if it's not even version specific, but rather a change in their docker images.

Perhaps we should be accepting the user/password we do today and using those to create a new user which is doled out to the fixture rather than the root user. I'll have to have a think about that, a much quicker fix ought to be what you suggest here ^, which is basically what i do in my PR

DanCardin commented 2 years ago

Hopefully fixed in 2.1.8

cobed95 commented 2 years ago

Thanks for the quick feedback. Is there any estimated release date for v2.1.8?

DanCardin commented 2 years ago

It's been released since my comment, though i see i forgot to actually do the github release

cobed95 commented 2 years ago

I see, thanks again!