django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

drop support for python 3.6 and 3.7

Open deronnax opened this issue 2 years ago • 5 comments

EDIT: improved rationale: DRF supports the currently supported Django releases. Django 3.2 upstream support will come to an end April 1st 2024, and then Django 4+ will only support python 3.8+, see endoflife.date for django.

To merge for the release we will do around April.

deronnax avatar Oct 09 '23 12:10 deronnax

Can I work on this issue? I am pretty new to this

jalajk24 avatar Jan 29 '23 20:01 jalajk24

A potential simpler solution could be a Django app setting that allows filenames to be automatically shortened if over the limit.

This avoids the complexity of an additional UI interaction but rather a decision by the developer to ignore this limitation.

It would be good for this to be added to images, not just documents.

Note. Intentionally not a Django setting but an app setting.

lb- avatar Jan 29 '23 20:01 lb-

@jalajk24 this may not be a good first issue, you would need to be familiar with Django app settings, images / documents in Wagtail (including the various ways files can be added). Also, would need to ensure that the existing 'title from filename' logic works the same.

Up to you if you feel confident giving this a go, however, some other core team may want to add their thoughts to this issue before you proceed. Maybe a good first step would be to draft up some POC code on your local set up then post a comment with the approach you think could be taken here.

lb- avatar Jan 29 '23 21:01 lb-

A potential simpler solution could be a Django app setting that allows filenames to be automatically shortened if over the limit.

This avoids the complexity of an additional UI interaction but rather a decision by the developer to ignore this limitation.

It would be good for this to be added to images, not just documents.

Note. Intentionally not a Django setting but an app setting.

what could be the potential character limit? 100 chars or less

salty-ivy avatar Feb 02 '23 09:02 salty-ivy

Hey, Is someone working on this issue or can I work on this?

Aravind2203 avatar Feb 05 '23 05:02 Aravind2203

@salty-ivy - the actual limit (restriction) number may come from Django. I think this issue needs a bit more investigation such as.

  • Why 100chars - where is the actual error coming from?
  • If we do make a setting, is that setting for the limit or simply the auto-concatenation functionality

Generally, I think someone needs to write up a proposal for how this could work, get buy in and then build it. As for whether anything should be done about this, I'm not sure, my suggestion above was just trying to provide some other ideas about how we could provide a nicer UX for users.

@Aravind2203 - no one is actively working on this, but as per discussion so far, it probably needs a bit more thought before a PR is created.

lb- avatar Feb 05 '23 05:02 lb-

@lb- The error does come from Django. Django restricts file name to be 100 characters. Need to check if there is a way to override that.

Aravind2203 avatar Feb 05 '23 06:02 Aravind2203

A potential simpler solution could be a Django app setting that allows filenames to be automatically shortened if over the limit.

This avoids the complexity of an additional UI interaction but rather a decision by the developer to ignore this limitation.

It would be good for this to be added to images, not just documents.

Note. Intentionally not a Django setting but an app setting.

@lb- how about, making it a default behavior to truncate characters over the limit of 100 chars (default by Django) and provide a settings to increase the character limits ?

salty-ivy avatar Feb 05 '23 07:02 salty-ivy

Would try to make a draft PR along with the proposal within it

salty-ivy avatar Feb 05 '23 07:02 salty-ivy

Go for it @salty-ivy

lb- avatar Feb 05 '23 07:02 lb-

The file field of AbstractDocument model does not have a max_length attribute.So django considers the default 100 char limit. Won't increasing the max_length solve the problem? @lb- @salty-ivy

Aravind2203 avatar Feb 05 '23 07:02 Aravind2203

Yea that's one potential approach however this would not solve the problem entirely, it still needs a proper structure and mechanics for customization and documentation for it You are welcome to investigate it thus it still requires more views from other members before it can be considered to accept any PRs

salty-ivy avatar Feb 05 '23 08:02 salty-ivy

Good investigation - increasing the limit could be done by anyone using Wagtail with a custom model. However, it's not super easy to add a custom image / document model for existing projects.

Maybe a partial solution is documentation that this limit can be changed this way and is part of Django's defaults.

Then an additional fix could be to allow opt-in (opt-out?) concatenation of that length.

Ideally though it would be nice to see if there is a simple way to add this behaviour currently without extra core code changes.

A reminder that we do have a way to change the title that gets generated clientside before it goes to the server, not sure of this clientside event listener could be extended to intercept the file name before it goes to the server also.

https://docs.wagtail.org/en/stable/advanced_topics/images/title_generation_on_upload.html#title-generation-on-upload

lb- avatar Feb 05 '23 08:02 lb-

@lb- I was wondering if it might be a good approach to match the current file name size with the default maximum file name size supported by various operating systems.

For reference, Windows, Linux and MacOS support maximum file name length of 255 characters.

I'm interested in exploring this idea further and ready to investigate if you have any specific approaches or suggestions in mind. Your input would be greatly appreciated!

rohitsrma avatar Oct 02 '23 10:10 rohitsrma

@rohitsrma good thinking, I feel like increasing the maximum is probably a simple solution and makes sense. Could you add some references to where you found this file length info.

We also need to be sure that this length is inclusive of extension and whether the extension is included in how the Django field works. We could maybe have a safe buffer (e.g. 200) if that's the case.

I think it's still worth looking into how to avoid an error by maybe concatenating the filename before doing the upload. This would probably only be possible for our JS driven bulk upload widget though.

I do not have much more to add beyond what's already been discussed in this issue though. If you find a simple solution that covers most users, that's a great way to go.

lb- avatar Oct 05 '23 11:10 lb-

@lb- https://github.com/wagtail/wagtail/blob/d9c8d9fff30d8436d1ed7878d00f50cb3006da91/wagtail/documents/models.py#L26

I think we can add a max_length attribute in FileField based on our requirements. also, I think it's a good idea to follow your suggestion and include an extra buffer of 200 characters for added flexibility.

I'm exploring the idea of concatenating the filename before uploading!

rohitsrma avatar Oct 06 '23 09:10 rohitsrma

OK thanks @rohitsrma

lb- avatar Feb 06 '24 00:02 lb-