jugmac00 / flask-reuploaded

File uploads for Flask.
https://github.com/jugmac00/flask-reuploaded
MIT License
63 stars 12 forks source link

Use sets for extension groups in extensions.py #127

Closed djmattyg007 closed 1 year ago

djmattyg007 commented 2 years ago

Using sets would have three advantages over tuples:

frozenset can be used to keep immutability.

jugmac00 commented 2 years ago

I have no strong feelings about that.

Any other opinion on this? Mabye @tabebqena

tabebqena commented 2 years ago

Thanks @djmattyg007

I think that this change will bring a small performance improvement. I don't know is it worth or not because the file uploading process is expected to be slow by nature and it is affected by unpredictable factors like the file size, bandwidth etc. I argue that this few milliseconds is unnoticeable in relation to the whole request time.

In other hand, we have a tight limitation which is This extension is a drop in replacement of 'flask-uploads' extension.

If we want to keep this promise and give the user a hassle-free transition we should be strict for any change. Because, some good changes may result in a bug that is not detectable by unit tests (even with 100% coverage ) this situation will frustrate the user.

In summary, we have another larger question: Is there is some test suite that can test compatibility with another package. If there is something that give a guarantee for this condition we can move with more freedom and give it a try. According to my knowledge, there is no such guarantee.

jugmac00 commented 1 year ago

@djmattyg007 Your arguments are all valid, but as @tabebqena stated, we try hard to stay compatible with Flask-Uploads and with a library this old, you never know what users did with your API/code and I do not want to break anybody's application.