-
Notifications
You must be signed in to change notification settings - Fork 2
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
All form document fields can have a default upload size limit #115
Conversation
self.max_size = kwargs.pop('max_size', None) | ||
|
||
# Define an upload limit size based on the app settings and the field settings | ||
absolute_upload_size_limit = getattr(settings, 'OSIS_DOCUMENT_MAX_UPLOAD_SIZE', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Au vu de la documentation README (cf. https://github.com/uclouvain/osis-document/pull/115/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R67) , la valeur par défaut est 5242880
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En fait, c'était un exemple dans la documentation et dans osis j'ai mis une autre valeur. Est-ce que tu veux que je change cela ? De plus, je pensais considérer le paramètre comme facultatif. Est-ce que tu préfères qu'il soit obligatoire ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non effectivement, il est préférable de ne pas avoir de valeur par default. Par contre, au niveau du README, peux-tu préciser que la valeur par defaut est NONE ? (default: None)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oui, j'ai ajouté cette information au README.
else: | ||
field_upload_size_limit = absolute_upload_size_limit | ||
|
||
self.max_size = field_upload_size_limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce qu'on ne peut pas écrire ça sur une ligne ?
self.max_size = min(
filter(lambda size: size is not None, [field_upload_size_limit, absolute_upload_size_limit])
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En fait c'est juste le cas où les deux valeurs sont à None
que cela pose problème (min([]
) retourne une ValueError
). Mais si on est sûr d'avoir la valeur dans les settings je peux simplifier effectivement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok ok je comprends mieux, laisse comme ça alors merci
No description provided.