-
-
Notifications
You must be signed in to change notification settings - Fork 165
Create missing symbolic links in a course templates directory when upgrading. #2757
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
Create missing symbolic links in a course templates directory when upgrading. #2757
Conversation
…grading. This finishes @Alex-Jordan's work in openwebwork#2750, and checks for missing symbolic links in a course's templates directory when checking if courses need upgrading, and creates the missing links when upgrading the course. If a symbolic link points to an incorrect location (as defined in the course environment), then the upgrade process attempts to delete the symbolic link and recreate it pointing to the correct location. Note that the check for permissions of the symbolic links has been removed. For symbolic links there are two permissions in consideration. Those of the link itself, and those of the link target. The permissions of the link itself cannot be changed. Even the linux command line `chmod` utility cannot do this (contrary to what Google AI wants to tell you, there is no `-h` option for `chmod`). Furthermore, linux completely ignores symbolic link permissions, and doesn't use them at all. So the only permissions that really matter are those of the link target which linux does use. However, the directories that are the target of these links do not belong to a particular course. They are system directories (or part of the webwork2 installation), and setting the correct permissions on those is part of the installation of webwork2. It should not be part of a course upgrade process. Including these permissions in the upgrade report when not correct would actually mean that every course would show that it has the incorrect permission, or no courses would. Upgrading would never fix this, because the permissions for these directories cannot be changed from the user interface in any case. What could be done here is to make another admin page that does a system status check, and lets the admin user know what directory permissions in the webwork installation are not correct what needs to be fixed. Of course the `setfilepermissions` script can be used to fix these permissions (it already handles most of the directories in question).
Thanks for picking this up. I will test it soon, perhaps this evening. |
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.
This worked for me to create missing links and change the target of a link that was bad.
One thing that might pair with this PR though. When using the Archive Courses tool on a course with bad directories or links, you get the options to "Attempt to upgrade directories or links".
But when you use Rename Course, you don't get that option. You are told you must manually repair the directories and/or links. Is there any reason not to have the option here too?
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.
Works well. I agree with @Alex-Jordan if it's not a heavy lift to handle the rename course to fix the links as well.
Also fix the logic both here and when archiving a course for the possible combinations of the directories, links, and database needing repair. With the previous logic if the directories were good, but the database tables and links both needed repair, then the button said "Attempt to upgrade directories and links". However, in actuality if that button were used, the server would both repair links and repair database tables. Now, anytime the database tables need repair the button says "Upgrade Course Tables" regardless of the state of the directories and links. This is not ideal since directories or links might also be repaired in this case, but the important thing to show is the database repair which is more important than directories and vastly more important than links.
Now there is an option to fix directories and links when renaming a course. Also the logic was fixed both for renaming and archiving a course for the possible combinations of the directories, links, and database needing repair. With the previous logic if the directories were good, but the database tables and links both needed repair, then the button said "Attempt to upgrade directories and links". However, in actuality if that button were used, the server would both repair links and repair database tables. Now, anytime the database tables need repair the button says "Upgrade Course Tables" regardless of the state of the directories and links. This is not ideal since directories or links might also be repaired in this case, but the important thing to show is the database repair which is more important than directories and vastly more important than links. |
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.
Looks good.
One thing I notice is I prefer relative symlinks vs absolute, but this is just due to my setup, for instance I would prefer ../../../libraries/webwork-open-problem-library/OpenProblemLibrary/
over /opt/webwork/libraries/webwork-open-problem-library/OpenProblemLibrary/
. I did test that the relative symlinks do get validated correctly, so this is just a very minor thing when creating the new links.
Though I guess this just falls into a more complicated setup and I can manually create links for my use case. Also any bad links can be fixed when moving a course to a different server with these new tools. Just sharing a random thought mostly.
With using the directories from the course environment it is not possible to use relative paths for the links. Actually, not using relative paths for the symbolic links is better than the current links in the model course anyway. Perhaps you like relative links, but using absolute paths solves a problem. That is that theoretically the location of the courses directory is configurable, as is the location of the OPL. So if someone uses a non-standard location for the courses directory or for the OPL, then the links in the model course won't work. However, the links created by this method will. It seems that what we really should do is remove the links from the model course, and when a course is created the links should be created from the configuration. Perhaps when unarchiving a course the links could even be created/fixed at that time. Then there would be no need for handling this in the course update, and the links would always be there and work. |
This finishes @Alex-Jordan's work in #2750. It checks for missing symbolic links in a course's templates directory when checking if courses need upgrading, and creates the missing links when upgrading the course. If a symbolic link points to an incorrect location (as defined in the course environment), then the upgrade process attempts to delete the symbolic link and recreate it pointing to the correct location.
Note that the check for permissions of the symbolic links has been removed. For symbolic links there are two permissions in consideration. Those of the link itself, and those of the link target. The permissions of the link itself cannot be changed. Even the linux command line
chmod
utility cannot do this (contrary to what Google AI wants to tell you, there is no-h
option forchmod
). Furthermore, linux completely ignores symbolic link permissions, and doesn't use them at all. So the only permissions that really matter are those of the link target which linux does use. However, the directories that are the target of these links do not belong to a particular course. They are system directories (or part of the webwork2 installation), and setting the correct permissions on those is part of the installation of webwork2. It should not be part of a course upgrade process. Including these permissions in the upgrade report when not correct would actually mean that every course would show that it has the incorrect permission, or no courses would. Upgrading would never fix this, because the permissions for these directories cannot be changed from the user interface in any case. What could be done here is to make another admin page that does a system status check, and lets the admin user know what directory permissions in the webwork installation are not correct what needs to be fixed. Of course thesetfilepermissions
script can be used to fix these permissions (it already handles most of the directories in question).@Alex-Jordan: Hopefully you are okay with me opening a new pull request instead of making a pull request to your branch. I thought this would be easier for me to maintain, since you said you didn't have time now to finish that pull request.