Fix boto3 >=1.36.0 breaks most non-AWS S3 providers

[wgresshoff]

❤️ Thank you for your contribution!

Fixes #35

Description

Fixes the checksum issue by setting default values for request_checksum_calculation and response_checksum_validation to 'WHEN_REQUIRED'.

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

• I'm aware of the code of conduct.
• I've created logical separate commits and followed the commit message format.
• I've added relevant test cases.
• I've added relevant documentation.
• I've marked translation strings.
• I've identified the copyright holder(s) and updated copyright headers for touched files (>15 lines contributions).
• I've NOT included third-party code (copy/pasted source code or new dependencies).
  • If you have added third-party code (copy/pasted or new dependencies), please reach out to an architect.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

1. You license your contribution under the same terms as the current repository’s license.
2. You agree that you have the right to license your contribution under the current repository’s license.

[egabancho]

Hey @wgresshoff, thanks a lot for taking a look into this!

If we add this to the default configuration, it will be detrimental to every instance running on AWS S3.

I'm inclined to think like @tmorrell, #35 (comment). I believe it might be better to document it somehow, either here or in the general docs (or both!)

What do you think?

[wgresshoff]

I didn't understand it that way! And I don't think so. This patch only allows setting the behaviour in invenio.cfg easily, so not having to put configuration to different places (when new ops people will have a hard time to find every bit of config!). But perhaps it's better to set the default to WHEN_SUPPORTED and let the users of non AWS S3 providers do the configuration?!
Either way, even setting the values to WHEN_REQUIRED just switches of the S3 integrity protection Amazon introduced just one or two months ago 😉

[wgresshoff]

Guessing more and more alternative S3 providers will support the new integrity protection it is better to set the default to the Amazon S3 default.

[egabancho]

This is great. Thank you for the changes.
I'd like your opinion on something before we merge, though.
Do you think it would be better to create a configuration variable, say S3_CONFIG_EXTRA, to allow passing not only these two variables but also any others you might need depending on the storage solution you're using?

[wgresshoff]

Good idea, that would significantly reduce further changes in the code.
But on the other side (and perhaps I'm wrong) this would be a complex configuration variable because you would need the key and the value to set and thus it would be impossible to put it in the environment.
And so far I don't know about any special configuration options only available for our storage solution. If I sum that up I'm still unsure but have a tendency to a soft "No, I don't think so".

[egabancho]

#37