Skip to content
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

Updated Azure storage compatibility #647

Open
wants to merge 8 commits into
base: MOODLE_402_STABLE
Choose a base branch
from

Conversation

matthewhilton
Copy link
Contributor

@matthewhilton matthewhilton commented Nov 5, 2024

Changes

  • Adds new azure_blob_storage_file_system class to replace the deprecated azure_file_storage_system.
    • This is intentionally added as an additional class (as opposed to renaming/fixing the old class) because the new class requires a new plugin dependency https://github.com/catalyst/moodle-local_azureblobstorage
    • Having a new class allows time for admins to install the new plugin in parallel, and then switch over rather than suddenly finding the newest minor version update changed dependencies.
  • Added documentation about migration process for the above
  • Fixes a tiny bug in a test that assumed no files existed before the test
  • Fixed a bug where testing deletion does not check if the fs even is allowed to delete, related to $testdelete setting in client does not respect plugin config, delete permissions checks may be outdated #645 but does not completely resolve the issue for other fs's like AWS

Testing

  • The new client class with the new sdk plugin pass the integration tests (the standard objectfs unit tests but attached to azure storage via the azure_blob_storage_file_system)
  • Also tested locally the various CRUD operations including large files uploaded via multipart upload and large files streamed down (e.g. videos) in multipart - all working as expected

@matthewhilton matthewhilton force-pushed the new-azure-sdk branch 2 times, most recently from 46b6323 to 6204daa Compare November 5, 2024 00:38
@Peterburnett
Copy link
Contributor

Should we be emitting debugging() anywhere when the old SDK is used? It has the potentially to be EXTREMELY noisy, just wondering if it would be useful with out drowning out noise.

@matthewhilton
Copy link
Contributor Author

Should we be emitting debugging() anywhere when the old SDK is used? It has the potentially to be EXTREMELY noisy, just wondering if it would be useful with out drowning out noise.

The old SDK already emits enough debugging messages on its own because of the php deprecation notices. I don't think we need to make it any worse and believe people will have enough reason to switch over when it becomes apparent the old one simply doesn't work anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants