-
Notifications
You must be signed in to change notification settings - Fork 31
Re-instate 'Fix aws library conflicts in Totara' #64
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
base: master
Are you sure you want to change the base?
Conversation
@Peterburnett Any chance we can move this one along? It impacts all Totara TXP sites and Moodle 4.2. |
sdk/aws-autoloader.php
Outdated
|
||
require __DIR__ . '/Aws/functions.php'; | ||
require __DIR__ . '/GuzzleHttp/functions_include.php'; | ||
if (!file_exists(__DIR__ . '/../../../../libraries/optional/aws/aws-sdk-php/src/functions.php') && !function_exists('Aws\constantly')) { |
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.
Replace the morse code with $CFG->libraries
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.
what stable branches is CFG->libraries supported on?
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.
It's in Totara 13+, so maybe just if (!isset($CFG->libraries) && [...]
would be more appropriate
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.
If so make sure the comment is clear saying exactly why its doing this and on what versions
ac7b8da
to
d7fff33
Compare
|
||
require __DIR__ . '/Aws/functions.php'; | ||
require __DIR__ . '/GuzzleHttp/functions_include.php'; | ||
if ((!isset($CFG->libraries) && !file_exists($CFG->libraries . '/optional/aws/aws-sdk-php/src/functions.php')) |
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.
if ((!isset($CFG->libraries) && !file_exists($CFG->libraries . '/optional/aws/aws-sdk-php/src/functions.php')) | |
if ((!isset($CFG->libraries) || !file_exists($CFG->libraries . '/optional/aws/aws-sdk-php/src/functions.php')) |
The AND
should probably be an OR
, otherwise the two statements are mutually inclusive.
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.
Been trying to do this in-between other work but I won't be able to review this week as I'm doing upgrade prep for the rest of the week then on leave next week, so could someone else pick it up perhaps? Happy to give access to my repo for any volunteers.
Librelambda plugin is also affected by this issue. Kindly reinstate asap. |
This commit is slightly different to the original as we encountered an issue today for a site that had a block that utilises elastic search added to a course page, for which the original fix did not successfully work.
This fix should be slightly more robust in that it checks for optional library file and the function; if those don't exist then it will use the plugin version.