Changes and fixes to symlinking and serving #2611
Conversation
agjohnson
added
the
PR: work in progress
agjohnson
force-pushed
the
symlink-translations
branch
from
52fe4d9 to
31459e1
Compare
agjohnson
force-pushed
the
symlink-translations
branch
from
31459e1 to
f4198a9
Compare
agjohnson
added
PR: ready for review
and removed
PR: work in progress
|
I feel like this stuff needs to be documented, as reading through it it's really hard to infer why it was designed this particular way. I'd suggest adding it to either a design or architecture doc in our actual documentation. |
| } | ||
| defaults.update(kwargs) | ||
| return self.create(**defaults) | ||
| def for_project(self, project): |
There was a problem hiding this comment.
This doesn't appear to be used anywhere?
There was a problem hiding this comment.
It's used on the overrides for rtd.com, but I believe the same bug exists in the .org as well. I'm testing it now.
There was a problem hiding this comment.
So, the serving bug would exist here, but because we are only serving public projects, it's can't be reproduced. The bug is most noticeable when there is a mismatch in privacy levels on versions.
| self.assertFilesystem(filesystem) | ||
|
|
||
|
|
||
| @override_settings() |
There was a problem hiding this comment.
Does this do anything when called without arguments? I thought it was used to set settings explicitly.
There was a problem hiding this comment.
Wrapping a class/method like this isolates changes to settings from leaking out. On decorating, setUp/tearDown additions are added and the standard settings._wrapped -- the underlying object on the django lazy settings object -- is temporarily replaced.
|
Agree on docs for extension pattern, but can deal with it in #2618 perhaps? |
|
Makes sense |
This allows for override of the symlinking classes, as we need to be able to
override how public and private symlinks are determined. Test coverage is expanded greatly on symlinking here:
Our extension pattern was tuned up and simplified here, the django lazy object didn't allow for passing of classmethods.
This also moves the Version manager into a Version queryset + manager pattern, as we need to be able to chain queries from the managers, which isn't possible without a queryset object. This is a pattern we should adopt on the remaining managers, but I didn't touch that here. I'll open a ticket to address this.