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

Update flysystem usage after upgrade #2584

Merged
merged 11 commits into from
Nov 28, 2024
Merged

Conversation

Copy link

github-actions bot commented Oct 10, 2024

Front-end summary Node 18

💯 Total ✅ Passed ⏭️ Skipped ❌ Failed
287 287 0 0

@@ -202,7 +202,7 @@ protected function isInstanceValid($item)
{
try {
$xml = Service::singleton()->getXmlByRdfItem($item);
} catch (FileNotFoundException $e) {
} catch (FilesystemException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some logging here as well?

foreach ($cssFiles as $key => $file) {
$cssFiles[$key]['stream'] = $this->getFileSystem()->readStream(
$stylesheetPath . DIRECTORY_SEPARATOR . $file['basename']
);
}

return $cssFiles;
} catch (FileNotFoundException $exception) {
} catch (FilesystemException $exception) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like that we bound to flysystem exception League\Flysystem\FilesystemException inside tao here and in some other places. Maybe it makes sense to create an internal tao exception and catch all Filesystem exceptions here FileSystemWrapperTrait and throw the tao exception instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, great suggestion, I've updated PRs to catch a filesystem exception defined in our application

Copy link

Version

❕ Some commits are not using the conventional commits formats. They will be ignored in version management.

Target Version 30.24.0
Last version 30.23.0

There are 0 BREAKING CHANGE, 1 feature, 4 fixes

@augustas augustas merged commit 81bf766 into develop Nov 28, 2024
5 checks passed
@augustas augustas deleted the feat/REL-1723/update-flysystem branch November 28, 2024 11:19
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.

3 participants