django-bakery icon indicating copy to clipboard operation
django-bakery copied to clipboard

Fix build command on Windows

Open danizen opened this issue 2 years ago • 6 comments

DESIRED BEHAVIOR

My team uses django-bakery in a couple of places. We want builds on Windows to just work without workarounds. However, right now we need some workarounds because of limitations that prevent builds on Windows.

WORKAROUND

In my environment so far, two changes are needed to allow builds on Windows to proceed.

First, make sure the BUILD_DIR is free of a colon, which fails validation by python-fs:

__build_dir = os.environ.get('BUILD_DIR', os.path.join(BASE_DIR, 'build'))
if sys.platform == 'win32':
    __build_dir = __build_dir.replace('\\', '/')[2:]
BUILD_DIR = __build_dir

Second, override the build method of BuildableTemplateView to correct for the use of os.path.join:

from bakery.views import BuildableTemplateView as BaseBuildableTemplateView


class BuildableTemplateView(BaseBuildableTemplateView):
    def build(self):
        logger.debug("Building %s" % self.template_name)
        build_path = self.get_build_path()
        self.request = self.create_request(build_path)
        path = fs.path.combine(settings.BUILD_DIR, build_path)
        self.prep_directory(build_path)
        self.build_file(path, self.get_content())

SHORT-TERM FIX

A short-term fix is to simply promote at least the 1-line change to build into the package. 3 lines in the local or development settings file is acceptably dry. I'll admit that our work-around for the BUILD_DIR is not a complete solution - you need the drive specification on Windows.

I will work on a pull request for the short-term fix, but I think there will need to be discussion around the longer term fix.

LONG-TERM FIX

Using python-fs is really not that great now that Python 3 supports the pathlib module. It might be possible to migrate to the pathlib module.

danizen avatar Feb 28 '22 16:02 danizen

I'm okay with hammering out a short term fix to get it working for you. I am open to deprecating python-fs in favor of pathlib.

palewire avatar Feb 28 '22 16:02 palewire

I just did a "python setup.py test", and it looks like some requirements would need to be pinned for me to test successfully. Some of these are in test code, and so pinning Django==2.2 might be acceptable to deal with this for now. I may need to do more changes than I expected.

danizen avatar Feb 28 '22 17:02 danizen

Is a live bucket required to run tests? Can that be easily parameterized?

danizen avatar Feb 28 '22 17:02 danizen

It's been a good while since I worked through these details, but I'm pretty sure Travis pulls it off with a mock.

https://github.com/palewire/django-bakery/blob/master/.travis.yml#L13-L49

palewire avatar Feb 28 '22 17:02 palewire

I see. The intention is to define AWS_ACCESS_KEY_ID and so on to make sure that we are not mutating any real environment, as described in moto documentation.

The issue is resolved for me with small changes to the tests, I think deriving from changes in how the moto API works - it patches now on client or resource creation I think. The boto API has also changed a little and I also had to use kwargs only in a couple of places in tests. Might not have needed to do if I got the mocking right first.

https://github.com/danizen/django-bakery/commit/4b655d83046ed5d3f7f083aa6d816a4d2404e58d

I am testing now only with Django 2.2, but I saw there were some issues with Django 4. I will give more recent versions of Django a try tomorrow.

danizen avatar Feb 28 '22 23:02 danizen

I have tested with Django 3.2 and added it to the build matrix. I am stopped from a good pull request by two things:

  • Travis-CI.org shutdown awhile ago, and so I would have to go through a lot to test that Python 2.7 still works. Dropping support for Python 2.7 and Django 1.11 would make me confident - without them I would want a build bot.

  • A better fix would not require a work-around on Windows for BUILD_DIR setting. So, I want to understand an OSFS is supposed to work on Windows. My intuition is that the BUILD_ROOT should be validated with os.path and then opened as a filesystem itself, and the relative path is what should be validated. However, I am not certain and want to wait for PyFilesystem/pyfilesystem2#526

danizen avatar Mar 01 '22 17:03 danizen