5.x Extended Discussion
Closed this issue · 13 comments
I've just saw that my refactor for v5 has been merged and tagged into a pre-release, and I'm pretty grateful about it. I think this is a great step towards a new major release.
However, I think there are still some things we could do before officially tagging v5.
Some ideas I have:
- Temp URL method should internally check for Temp URL Key existence, otherwise throw an error.
- Implement a Connection and/or Container switcher by default (Related to my comment on #47)
Another possible feature that I would like to have would be to implement a Form Post signature generator for uploading files directly to OVH's Object Storage. It could help a lot by preventing the use of server resources to re-upload files to OVH. Inspiration from Laravel Vapor. This would need a new JS package to mimic what Vapor can do, but for OVH. This would be an opt-in feature.
Any thoughts @sausin?
BTW: Could yo disable StyleCI on their dashboard? It keeps triggering after I removed their config file from the repo on 5.x refactor PR...
@iksaku good things are happening in the package, 5.x is thanks to you 😃. Marked it pre-release because lots of people might be thrown off by the changes in the last few days and might have things falling apart.
Regarding the Temp URL command, do you mean we should check if the container is private or not? I couldn't find a way to get this information through the container. Had opened an issue on the openstack repo, but no progress so far.
If you know of a way to do it, I think throwing an error would be the correct thing to do. If you meant something else, please let me know!
Connection and container switching looks like a good option to have. I think we should do it for both.
Regarding the Form Post, this looks great and I hadn't read about it! As I understand it, the backend just needs to pass a signature that can be used by the frontend (reasonably short expiry by default?) to upload files directly to a supported container. Would be a significant boost in saving server resources!
As per the linked page, it says one should make a check if the OS system supports this feature. If you've tried it, then it's good to go ahead. The features page doesn't list it and I haven't found any documentation around it.
PS:- had forgotten to disable StyleCI through the dashboard. It is now done!
About Temp URL Key Check
I didn’t mean to check if the container was private or not, sorry for the vague explanation.
Let me elaborate a bit more... Currently, we don’t check against the existence of a registered Temp URL Key in the config (could be or not present in .env file). What this means is that the hash_hmac function could generate a signature based on an empty key. We should implement a check at the beginning of the getTemporaryURL function. Will try to send a PR tomorrow for this.
About Connection and Container switching
Maybe, we could implement a Container switcher by ourselves, and rely on Laravel’s Storage::disk() method for full connection switch. This way we can only focus on keeping the container switcher working for connections with the same credentials, and encourage the use of Storage::disk() for switching credentials.
About Form Post
You could send a GET to /info endpoint in your container to get the available features for the Openstack provider itself.
I’ve previously checked and OVH does support Form Post, although not documented nor mentioned by them, but listed in the provider features endpoint.
Also, just got some sample code running following Openstack’s documentation. It was kinda tricky to get, but it works. I’ll keep looking more into it during the following week, hopefully I can get a demo working.
Looks good on all three fronts!
We could specify in our docs that the Form Post feature is undocumented by OVH but works and that we don't know if they intend to support it. Use it at your own risk kind of 😃
Using the python-swiftclient CLI client, I've run the swift info command, which outputs the following (redacted) information:
Core: swift
Options:
account_autocreate: True
account_listing_limit: 10000
allow_account_management: True
container_listing_limit: 10000
extra_header_count: 0
max_account_name_length: 256
max_container_name_length: 256
max_file_size: 5368709122
max_header_size: 8192
max_meta_count: 90
max_meta_name_length: 128
max_meta_overall_size: 4096
max_meta_value_length: 256
max_object_name_length: 1024
policies: [{'name': 'PCS', 'default': True, 'aliases': 'PCS'}, {'name': 'PCA', 'aliases': 'PCA'}]
strict_cors_mode: True
valid_api_versions: ['v1', 'v1.0']
version: 2.22.1.dev99
Additional middleware: formpost
Additional middleware: s3api
Options:
allow_multipart_uploads: True
max_bucket_listing: 1000
max_multi_delete_objects: 1000
max_parts_listing: 1000
max_upload_part_num: 1000
min_segment_size: 5242880
s3_acl: FalseAs stated in the very first paragraph of Openstack's Form Post Documentation, a GET request to the /info endpoint on the Object Storage provider (equivalent to swift info command) will output the available features given by the service provider. If the form post middleware is available, then the feature can be used.
As we can see in the response to my request to OVH, there is indeed support form Form Post middleware, as well as support for S3-Compatibility-Layer API, which we could also use (Similar to Laravel Vapor).
As we're running for OVH exclusively, I suggest to stick to Form Post middleware. I would like to implement a custom Singleton Object that provides access to Signature Generation for Form Post, and maybe could also move the Temp Url Signature to that same Singleton.
What do think @sausin?
Temp URL method should internally check for Temp URL Key existence, otherwise throw an error.
Implemented in #50
Temp URL method should internally check for Temp URL Key existence, otherwise throw an error.
Implemented in #50
Nice!! Will take a look and post any progress in the PR 😃
As we can see in the response to my request to OVH, there is indeed support form Form Post middleware, as well as support for S3-Compatibility-Layer API, which we could also use (Similar to Laravel Vapor).
As we're running for OVH exclusively, I suggest to stick to Form Post middleware. I would like to implement a custom Singleton Object that provides access to Signature Generation for Form Post, and maybe could also move the Temp Url Signature to that same Singleton.
What do think @sausin?
A direct request curl --silent -G https://storage.GRA.cloud.ovh.net/info | json_pp also results in similar info (below). So it's quite clear that we should be okay to use it! The thoughts behind using a Singleton make sense to me. PR is welcome 😃
{
"some": "info.......",
"tempurl" : {
"outgoing_allow_headers" : [
"x-object-meta-public-*"
],
"outgoing_remove_headers" : [
"x-object-meta-*"
],
"incoming_remove_headers" : [
"x-timestamp"
],
"allowed_digests" : [
"sha1",
"sha256",
"sha512"
],
"incoming_allow_headers" : [],
"methods" : [
"GET",
"HEAD",
"PUT",
"POST",
"DELETE"
]
},
"formpost" : {},
"some": "info.......",
}Was thinking some more regarding the connection switcher. When using the command to set temp url key, it might be useful to just list out the drivers available from the filesystems config for the user to select.
Similar to the options you get when performing php artisan vendor:publish
What do you think?
When using the command to set temp url key, it might be useful to just list out the drivers available from the
filesystemsconfig for the user to select.Similar to the options you get when performing
php artisan vendor:publish
Sounds pretty good! There should be a way to detect which disks use the OVH driver, and list them there.
A simple code block like below gives the available disks which are for this package:
array_keys(array_filter(Config::get('filesystems.disks'), function ($d) {
return $d['driver'] === 'ovh';
}));Hopefully this improves convenience!
The last bit is now in, closing 👍