Fix #2476 #2477
Fix #2476 #2477
Conversation
|
This goes to develop as it is part of a larger set of hopefully smaller fixes to make archiving work again. |
|
it can't be based on stable without merge conflicts |
| @@ -1788,7 +1788,7 @@ def archive_operation(self): | |||
| "token": self.token, | |||
| "op_id": self.active_op_id, | |||
| # when a user archives an operation we set the max “natural” integer in days | |||
| "days": sys.maxsize, | |||
| "days": 99999, # larger values run into a lot of issues | |||
There was a problem hiding this comment.
Unfortunately someone with an old version will set the value where you see a lot of issues. This can be solved when we change the server code, the server has to do where we used sys.maxsize always your new value.
We likly have to deprecate the API that the client sends the days at all.
Because the user can't alter the value we should force the value on the server, and remove the form data of days
please fix it in the server code
https://github.com/Open-MSS/MSS/blob/develop/mslib/mscolab/server.py#L678
And on a migration script for the database we can adjust that number too.
Please add a ToDo or issue as follow up.
There was a problem hiding this comment.
I do not understand the migration scripts in mss/mslib/mscolab/migrations/versions in particular the hashes? What do they relate to. I do not find them in git log.
I am also not sure that a fix is necessary as the server will crash before setting an illegal date, so the dates in the database should be fine.
So this can probably be merged as is.
There was a problem hiding this comment.
The server checks the value supplied by client and truncates the value. So old clients should work as well. "Normal" legit use for days>99999 are not possible.
There was a problem hiding this comment.
I do not understand the migration scripts in mss/mslib/mscolab/migrations/versions in particular the hashes? What do they relate to. I do not find them in git log. I am also not sure that a fix is necessary as the server will crash before setting an illegal date, so the dates in the database should be fine. So this can probably be merged as is.
In the database the hashes are set and connected by that to the scripts.
There was a problem hiding this comment.
OK, the link gives some context, but I still have issues with that. However, this PR does not change the schema, thus flask db migrate does not generate a script and this script does not even affect values currently in the database, where datetimes are stored not the values being communicated via API.
So, I will look into migration for the other draft PR and this can be merged as it is.
|
For the time based there is also an ARCHIVE_THRESHOLD defined, which can be altered on the server config. https://github.com/Open-MSS/MSS/blob/develop/mslib/mscolab/conf.py#L44 |
? Yes, this MR fix does not change that and should use this variable. |
Purpose of PR?:
Fixes #2476
Does this PR introduce a breaking change?
no