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

Windows path fix: should do like Django, not like Windows #29

Closed
SonOfLilit opened this issue Jul 19, 2016 · 1 comment · Fixed by #198
Closed

Windows path fix: should do like Django, not like Windows #29

SonOfLilit opened this issue Jul 19, 2016 · 1 comment · Fixed by #198

Comments

@SonOfLilit
Copy link

In March I was in a hurry and I locally fixed a path issue where Django always worked with '/' but db_file_storage worked with os.sep (I fixed it locally because it was above the @sp-niemand fork that fixed Django 1.9, so contributing the fix was not easy anyway.)

This is my fix:

https://github.com/SonOfLilit/db_file_storage/commits/master

I recently noticed that the issue was fixed upstream (as well as the Django 1.9 support that was blocking me from using the official release):

#28

However, when I tested the new version, I found that the official fix converted Django's forward slashes to backslashes on Windows, instead of using forward slashes internally on Windows too like Django does, and like a db-based storage plugin should, so I'm stuck on my unofficial version.

The abstract issue is that there is no relation between a storage plugin and the operating system, so why should the storage plugin introduce a portability issue between operating systems, or depend on the operating system for it's API/ABI in any way?

An example concrete issue is that I can't clone the production DB and work on it from a Windows machine. Few people ever host production Django systems on a Windows server [citation needed], so dev environments for systems that are hosted on Unix in production is the major use case for using Django on Windows, and therefore this matters.

I propose moving to my fix and creating a migration that converts all existing paths in the DB to forward slashes (perhaps only do it if there are slashes in the config part of the stored paths... Windows does not allow backslashes in a filename, does Unix?).

I can write the code if you like the idea and are willing to do Code Review.

Aur

@Aagam41
Copy link
Owner

Aagam41 commented Jan 16, 2024

Hello @SonOfLilit,
I'm currently maintaining this project.

I believe I've found a solution that could work. However, I'd love to collaborate with you and review the solution you've come up with. I believe that by combining our efforts, we can create the best possible solution for our open-source project.

Could you please share the details of your solution or submit a pull request? I'm eager to dive into the code, discuss potential improvements, and ensure that we make the most informed decision for the project.

Your contributions are highly valued, and I appreciate your effort in addressing this issue. Looking forward to our collaboration!

Warmest Regards,
Aagam Sheth

Aagam41 added a commit that referenced this issue Jan 20, 2024
Windows uses '\' (back-slash) rather than '/' (forward-slash) in its filepaths. This commit resolves this issue by replacing to correct slash on the operating system.

Resolves #29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants