djangoproject.com icon indicating copy to clipboard operation
djangoproject.com copied to clipboard

Fix #1817 docker dx

Open pbratkowski opened this issue 1 year ago • 4 comments

GitHub closed my previous PR (#1819) after I renamed a branch, so here is a new one.

Thanks to @tobiasmcnulty for pointing out a flaw in my previous implementation. This uses a build argument to determine when to remove packages, which allows removing them in the production build, but keeping them during local development which allows tox to build psycopg successfully in docker.

This also adds a container for the Trac database, which eliminates some errors upon startup, and makes tests pass in docker in most situations, and eliminates a lot of redundancy from the Docker settings file.

Fixes #1817

pbratkowski avatar Dec 16 '24 10:12 pbratkowski

Tests currently still fail if PARENT_HOST is set to a non-default value, e.g. if running on a LAN rather than on localhost.

The issue with those is that they use hardcoded domains, e.g. https://github.com/django/djangoproject.com/blob/ff2fd01dba5de6b0d641e86df9bba64f4770e8f9/docs/tests.py#L377-L385

Replacing the hardcoded values with something like the following makes them pass successfully:

    def test_sitemap_index(self):
        response = self.client.get(
            "/sitemap.xml", headers={"host": f"docs.{settings.PARENT_HOST}"}
        )
        self.assertContains(response, "<sitemap>", count=2)
        self.assertContains(
            response,
            f"<loc>http://docs.{settings.PARENT_HOST}/sitemap-en.xml</loc>",
        )

If anyone has alternative solutions, let me know.

pbratkowski avatar Dec 16 '24 10:12 pbratkowski

What's the value in keeping the dependencies in Docker?

Sorry, I should have added it to the description (which I have now updated). As mentioned in #1817 tox fails to install psycopg in Docker when the build dependencies are purged. By setting the argument in docker-compose.yml, this allows running tests in local dev environments.

pbratkowski avatar Dec 16 '24 14:12 pbratkowski

@pauloxnet how granular would you like the changes to be? I have removed the commit that fixes tests when settings.PARENT_HOST is set.

pbratkowski avatar Dec 17 '24 08:12 pbratkowski

I converted this back to a Draft for now, as we are working out smaller scope PRs.

pbratkowski avatar Dec 19 '24 14:12 pbratkowski