Fix Guzzle conflicts in Moodle 4.2

[sarahjcotton]

Fixes #56

[sarahjcotton]

Not sure why the master branch is failing with the error Not enough arguments (missing: "plugin").

[dmitriim]

hey @sarahjcotton Thanks for working on this!

There is an error in install Moodle step. Not your fault. Probably CI worflows should be updated and DB version bumped.

  RUN  php -d log_errors=1 -d error_log=NULL /home/runner/work/moodle-local_aws/moodle-local_aws/moodle/admin/tool/phpunit/cli/util.php --install
    OUT  == Environment ==
    OUT  !! database postgres (12.14 (Debian 12.14-1.pgdg110+1)) !!
    OUT  [System] version 13 is required and you are running 12.14 - 
    OUT  
    OUT  
    RES  1 Command did not run successfully
  
  In Execute.php line 180:
                                                                                 
    [Symfony\Component\Process\Exception\ProcessFailedException]                 
    The command "php -d log_errors=1 -d error_log=NULL /home/runner/work/moodle  
    -local_aws/moodle-local_aws/moodle/admin/tool/phpunit/cli/util.php --instal  
    l" failed.                                                                   
                                                                                 
    Exit Code: 1(General error)                                                  
                                                                                 
    Working directory: /home/runner/work/moodle-local_aws/moodle-local_aws       
                                                                                 
    Output:                                                                      
    ================                                                             
    == Environment ==                                                            
    !! database postgres (12.14 (Debian 12.14-1.pgdg110+1)) !!                   
    [System] version 13 is required and you are running 12.14

[sarahjcotton]

Thanks for fixing the workflow @keevan - Do you know when this PR is likely to get merged? We're in the middle of doing some upgrades which this is having an impact on. Many thanks.

[aspark21]

any update on this?

[dmitriim]

@sarahjcotton looks like this ready to be merged. Just wondering if there is a Guzzle version difference between core and this plugin?

[sarahjcotton]

@dmitriim From what I can tell the guzzle library included in 4.2 has deprecated the const VERSION and replaced it with MAJOR_VERSION in lib/guzzlehttp/guzzle/src/ClientInterface.php so yes, the versions are different.
This fix forces local_aws to use its own library and not the Moodle one.

[dmitriim]

> This fix forces local_aws to use its own library and not the Moodle one.

You mean opposite? Use Moodle one for 4.2+ ? Or am I missing something?

[sarahjcotton]

Yes that's correct - if the branch is 402 or greater then use the Moodle one, otherwise use the local_aws one.

[aspark21]

this works fine once on Moodle 4.2 but just doing a 3.11 to 4.2 upgrade and this errors out as the branch check doesn't sees itself as 4.2 yet, not really how I expected that check to behave

[sarahjcotton]

I'm wondering if it might be because the value is a string and I've done a check for an int (>=), so the = part would work for 4.2 but not anything else. I'm working on something else now but someone else should be along soon.

Edit: Probably really basic stuff I should already know but >= works on strings, so that's not it

[vlasov01]

Hi @sarahjcotton ,
Thank you for your fix. Could you please let me know when you think your fix will be merged?

[sarahjcotton]

I've changed this slightly so that if $CFG->branch >= 402 it always uses the plugin library as opposed to checking whether the function exists first. Otherwise if the core library function has been loaded first it won't move onto the branch check.

[mark-webster-catalyst]

Confirmed this fixed local_aws in Moodle 4.2. @aspark21 can you confirm upgrade from 3.11 now works? (I was unable to reproduce the error, even before the updated patch)

Edit: Found an instance where upgrade fails. It still fails with the latest patch.

[mark-webster-catalyst]

I'm not sure why those checks failed, bug in the workflow?

[marxjohnson]

We have this running on a client Moodle 4.3 site and have confirmed it is working.

[aspark21]

we’re using a forked version of this plugin in prod since it’s taken so long to get this fixed upstream (I’m assuming we’re the 4.3 client you were referring to) and we’re on a 4.2+ only version of the code with no version checks (the branch in #60)

Sorry for not having much time to re-test this but the upgrade steps that failed were for things that were updating files during the upgrade. e.g. format_grid was doing some conversions of icons

the code worked fine after the upgrade, it was just during the upgrade that it had problems.

[marxjohnson]

It's actually another site where we have this exact patch, and we're about to be upgrading some more, so hopefully we shouldn't see any further problems.