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

Refactor reload_openzaak_fixture to use TestContextDecorator #208

Open
SilviaAmAm opened this issue Jul 23, 2024 · 2 comments · May be fixed by #688
Open

Refactor reload_openzaak_fixture to use TestContextDecorator #208

SilviaAmAm opened this issue Jul 23, 2024 · 2 comments · May be fixed by #688
Assignees
Labels
enhancement New feature or request

Comments

@SilviaAmAm
Copy link
Collaborator

SilviaAmAm commented Jul 23, 2024

Suggested by Victorien. I had tried, but for some reasons the tests started hanging in CI. I didn't have the time to investigate it further.

Decorator: https://github.com/maykinmedia/open-archiefbeheer/blob/feature/165-destroy-cases/backend/src/openarchiefbeheer/utils/utils_decorators.py

Docs: https://github.com/django/django/blob/cf03aa4e94625971852a09e869f7ee7c328b573f/django/test/utils.py#L384

Also fix that in development mode it checks if we are running from cassettes

@SilviaAmAm SilviaAmAm added the enhancement New feature or request label Jul 23, 2024
@SilviaAmAm SilviaAmAm changed the title Refactor the decorator to reload OpenZaak fixtures to use TestContextDecorator Refactor reload_openzaak_fixture to use TestContextDecorator Jul 23, 2024
@SilviaAmAm SilviaAmAm self-assigned this Jul 26, 2024
@SilviaAmAm SilviaAmAm moved this from Todo to In Progress in Open Archiefbeheer - Sprints Feb 10, 2025
@SilviaAmAm
Copy link
Collaborator Author

SilviaAmAm commented Feb 10, 2025

The reason this is happening is:

  • When making the decorator a TestContextDecorator, VCR has already initialised mocks for getting a HTTP connection ( here )
  • The mocked _get_conn method of VCR, has a while loop. ( here ) This while loop expects that at some point the max number of connections for the pool will be reached:
# We need to make sure that we are actually providing a
# patched version of the connection class. This might not
# always be the case because the pool keeps previously
# used connections (which might actually be of a different
# class) around. This while loop will terminate because
# eventually the pool will run out of connections.
while not isinstance(connection, connection_class):
       connection = get_conn(pool, timeout)
  • This loop never seems to end.

I am not yet sure why. Docker-py makes HTTP connections without increasing the counter for the max number of connections:
This is docker-py ( here )

def _new_conn(self):
        return UnixHTTPConnection(
            self.base_url, self.socket_path, self.timeout
        )

While this is the original urllib3 HTTPConnectionPool ( here ) which docker-py overrides:

def _new_conn(self) -> BaseHTTPConnection:
        """
        Return a fresh :class:`HTTPConnection`.
        """
        self.num_connections += 1
        ...

However changing that function to increase the num_connections doesn't fix the problem. For some reason the pool queue is empty, so it keeps creating new connections.

Edit:
Found the real problem. VCR patches the urllib3 HTTPConnectionPool methods _get_conn and _new_conn. However, in the case of Dockerpy, we have UnixHTTPConnectionPool which inherits of HTTPConnectionPool but overrides the _new_conn method. Because of this, the loop in vcr:

while not isinstance(connection, connection_class):
       connection = get_conn(pool, timeout)

get_conn sees if there is an existing connection, and if there isn't it calls the _new_conn. Since the _new_conn method is not patched as expected, we always have a connection instance of UnixHTTPConnection and connection_class being VCRRequestsHTTPConnection.

So the loop never ends.

@SilviaAmAm
Copy link
Collaborator Author

In conclusion, I think it is not ideal to make this refactor, since the TestContextDecorator runs after that the VCR mocks have been initialised. Even after the fix to the problem with the infinite loop, the mocked VCR responses don't have the right shape that is expected by the docker client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Has Pull Request
Development

Successfully merging a pull request may close this issue.

1 participant