[REF] fs_storage: server_environment should not be a required dependency

Author: yankinmax

fs_attachment/tests/test_fs_storage.py

@@ -387,6 +387,7 @@ def test_url_for_image_dir_optimized_and_not_obfuscated(self):
             {
                 "name": "FS Product Image Backend",
                 "code": "file",
+                "protocol": "odoofs",
                 "base_url": "https://localhost/images",
                 "optimizes_directory_path": True,
                 "use_filename_obfuscation": False,

fs_storage/README.rst

@@ -121,8 +121,6 @@ When you create a new backend, you must specify the following:
 * The protocol options. These are the options that will be passed to the
   fsspec python package when creating the filesystem. These options depend
   on the protocol used and are described in the fsspec documentation.
-* Resolve env vars. This options resolves the protocol options values starting
-  with $ from environment variables
 * Check Connection Method. If set, Odoo will always check the connection before using
   a storage and it will remove the fs connection from the cache if the check fails.
 
@@ -153,29 +151,6 @@ the protocol options as follows:
 In this example, the SimpleCacheFileSystem protocol will be used as a wrapper
 around the odoofs protocol.
 
-Server Environment
-~~~~~~~~~~~~~~~~~~
-
-To ease the management of the filesystem storages configuration accross the different
-environments, the configuration of the filesystem storages can be defined in
-environment files or directly in the main configuration file. For example, the
-configuration of a filesystem storage with the code `fsprod` can be provided in the
-main configuration file as follows:
-
-.. code-block:: ini
-
-  [fs_storage.fsprod]
-  protocol=s3
-  options={"endpoint_url": "https://my_s3_server/", "key": "KEY", "secret": "SECRET"}
-  directory_path=my_bucket
-
-To work, a `storage.backend` record must exist with the code `fsprod` into the database.
-In your configuration section, you can specify the value for the following fields:
-
-* `protocol`
-* `options`
-* `directory_path`
-
 Migration from storage_backend
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

fs_storage/__manifest__.py

@@ -5,14 +5,14 @@
 {
     "name": "Filesystem Storage Backend",
     "summary": "Implement the concept of Storage with amazon S3, sftp...",
-    "version": "16.0.1.3.6",
+    "version": "16.0.1.3.7",
     "category": "FS Storage",
     "website": "https://github.com/OCA/storage",
     "author": " ACSONE SA/NV, Odoo Community Association (OCA)",
     "license": "LGPL-3",
     "development_status": "Beta",
     "installable": True,
-    "depends": ["base", "base_sparse_field", "server_environment"],
+    "depends": ["base", "base_sparse_field"],
     "data": [
         "views/fs_storage_view.xml",
         "security/ir.model.access.csv",

fs_storage/models/fs_storage.py

@@ -88,7 +88,6 @@ def wrapper(self, *args, **kwargs):
 
 class FSStorage(models.Model):
     _name = "fs.storage"
-    _inherit = "server.env.mixin"
     _description = "FS Storage"
 
     __slots__ = ("__fs", "__odoo_storage_path")
@@ -142,15 +141,6 @@ def __init__(self, env, ids=(), prefetch_ids=()):
         inverse="_inverse_json_options",
     )
 
-    eval_options_from_env = fields.Boolean(
-        string="Resolve env vars",
-        help="""Resolve options values starting with $ from environment variables. e.g
-            {
-                "endpoint_url": "$AWS_ENDPOINT_URL",
-            }
-            """,
-    )
-
     directory_path = fields.Char(
         help="Relative path to the directory to store the file"
     )
@@ -191,24 +181,13 @@ def __init__(self, env, ids=(), prefetch_ids=()):
         ),
     ]
 
-    _server_env_section_name_field = "code"
-
     @api.model
     def _get_check_connection_method_selection(self):
         return [
             ("marker_file", _("Create Marker file")),
             ("ls", _("List File")),
         ]
 
-    @property
-    def _server_env_fields(self):
-        return {
-            "protocol": {},
-            "options": {},
-            "directory_path": {},
-            "eval_options_from_env": {},
-        }
-
     @api.model_create_multi
     @prevent_call_from_safe_eval("create")
     def create(self, vals_list):
@@ -406,32 +385,10 @@ def _recursive_add_odoo_storage_path(self, options: dict) -> dict:
             self._recursive_add_odoo_storage_path(target_options)
         return options
 
-    def _eval_options_from_env(self, options):
-        values = {}
-        for key, value in options.items():
-            if isinstance(value, dict):
-                values[key] = self._eval_options_from_env(value)
-            elif isinstance(value, str) and value.startswith("$"):
-                env_variable_name = value[1:]
-                env_variable_value = os.getenv(env_variable_name)
-                if env_variable_value is not None:
-                    values[key] = env_variable_value
-                else:
-                    values[key] = value
-                    _logger.warning(
-                        "Environment variable %s is not set for fs_storage %s.",
-                        env_variable_name,
-                        self.display_name,
-                    )
-            else:
-                values[key] = value
-        return values
-
     def _get_fs_options(self):
-        options = self.json_options
-        if not self.eval_options_from_env:
-            return options
-        return self._eval_options_from_env(self.json_options)
+        # We need this hook to be able to override
+        # the options in the dependent modules
+        return self.json_options
 
     def _get_filesystem(self) -> fsspec.AbstractFileSystem:
         """Get the fsspec filesystem for this backend.

fs_storage/readme/USAGE.rst

@@ -15,8 +15,6 @@ When you create a new backend, you must specify the following:
 * The protocol options. These are the options that will be passed to the
   fsspec python package when creating the filesystem. These options depend
   on the protocol used and are described in the fsspec documentation.
-* Resolve env vars. This options resolves the protocol options values starting
-  with $ from environment variables
 * Check Connection Method. If set, Odoo will always check the connection before using
   a storage and it will remove the fs connection from the cache if the check fails.
 
@@ -47,29 +45,6 @@ the protocol options as follows:
 In this example, the SimpleCacheFileSystem protocol will be used as a wrapper
 around the odoofs protocol.
 
-Server Environment
-~~~~~~~~~~~~~~~~~~
-
-To ease the management of the filesystem storages configuration accross the different
-environments, the configuration of the filesystem storages can be defined in
-environment files or directly in the main configuration file. For example, the
-configuration of a filesystem storage with the code `fsprod` can be provided in the
-main configuration file as follows:
-
-.. code-block:: ini
-
-  [fs_storage.fsprod]
-  protocol=s3
-  options={"endpoint_url": "https://my_s3_server/", "key": "KEY", "secret": "SECRET"}
-  directory_path=my_bucket
-
-To work, a `storage.backend` record must exist with the code `fsprod` into the database.
-In your configuration section, you can specify the value for the following fields:
-
-* `protocol`
-* `options`
-* `directory_path`
-
 Migration from storage_backend
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

fs_storage/static/description/index.html

@@ -438,23 +438,22 @@ <h1>Filesystem Storage Backend</h1>
 <ul class="simple">
 <li><a class="reference internal" href="#usage" id="toc-entry-1">Usage</a><ul>
 <li><a class="reference internal" href="#configuration" id="toc-entry-2">Configuration</a></li>
-<li><a class="reference internal" href="#server-environment" id="toc-entry-3">Server Environment</a></li>
-<li><a class="reference internal" href="#migration-from-storage-backend" id="toc-entry-4">Migration from storage_backend</a></li>
+<li><a class="reference internal" href="#migration-from-storage-backend" id="toc-entry-3">Migration from storage_backend</a></li>
 </ul>
 </li>
-<li><a class="reference internal" href="#known-issues-roadmap" id="toc-entry-5">Known issues / Roadmap</a></li>
-<li><a class="reference internal" href="#changelog" id="toc-entry-6">Changelog</a><ul>
-<li><a class="reference internal" href="#section-1" id="toc-entry-7">16.0.1.2.0 (2024-02-06)</a></li>
-<li><a class="reference internal" href="#section-2" id="toc-entry-8">16.0.1.1.0 (2023-12-22)</a></li>
-<li><a class="reference internal" href="#section-3" id="toc-entry-9">16.0.1.0.3 (2023-10-17)</a></li>
-<li><a class="reference internal" href="#section-4" id="toc-entry-10">16.0.1.0.2 (2023-10-09)</a></li>
+<li><a class="reference internal" href="#known-issues-roadmap" id="toc-entry-4">Known issues / Roadmap</a></li>
+<li><a class="reference internal" href="#changelog" id="toc-entry-5">Changelog</a><ul>
+<li><a class="reference internal" href="#section-1" id="toc-entry-6">16.0.1.2.0 (2024-02-06)</a></li>
+<li><a class="reference internal" href="#section-2" id="toc-entry-7">16.0.1.1.0 (2023-12-22)</a></li>
+<li><a class="reference internal" href="#section-3" id="toc-entry-8">16.0.1.0.3 (2023-10-17)</a></li>
+<li><a class="reference internal" href="#section-4" id="toc-entry-9">16.0.1.0.2 (2023-10-09)</a></li>
 </ul>
 </li>
-<li><a class="reference internal" href="#bug-tracker" id="toc-entry-11">Bug Tracker</a></li>
-<li><a class="reference internal" href="#credits" id="toc-entry-12">Credits</a><ul>
-<li><a class="reference internal" href="#authors" id="toc-entry-13">Authors</a></li>
-<li><a class="reference internal" href="#contributors" id="toc-entry-14">Contributors</a></li>
-<li><a class="reference internal" href="#maintainers" id="toc-entry-15">Maintainers</a></li>
+<li><a class="reference internal" href="#bug-tracker" id="toc-entry-10">Bug Tracker</a></li>
+<li><a class="reference internal" href="#credits" id="toc-entry-11">Credits</a><ul>
+<li><a class="reference internal" href="#authors" id="toc-entry-12">Authors</a></li>
+<li><a class="reference internal" href="#contributors" id="toc-entry-13">Contributors</a></li>
+<li><a class="reference internal" href="#maintainers" id="toc-entry-14">Maintainers</a></li>
 </ul>
 </li>
 </ul>
@@ -477,8 +476,6 @@ <h3><a class="toc-backref" href="#toc-entry-2">Configuration</a></h3>
 <li>The protocol options. These are the options that will be passed to the
 fsspec python package when creating the filesystem. These options depend
 on the protocol used and are described in the fsspec documentation.</li>
-<li>Resolve env vars. This options resolves the protocol options values starting
-with $ from environment variables</li>
 <li>Check Connection Method. If set, Odoo will always check the connection before using
 a storage and it will remove the fs connection from the cache if the check fails.<ul>
 <li><tt class="docutils literal">Create Marker file</tt> : create a hidden file on remote and then check it exists with
@@ -507,29 +504,8 @@ <h3><a class="toc-backref" href="#toc-entry-2">Configuration</a></h3>
 <p>In this example, the SimpleCacheFileSystem protocol will be used as a wrapper
 around the odoofs protocol.</p>
 </div>
-<div class="section" id="server-environment">
-<h3><a class="toc-backref" href="#toc-entry-3">Server Environment</a></h3>
-<p>To ease the management of the filesystem storages configuration accross the different
-environments, the configuration of the filesystem storages can be defined in
-environment files or directly in the main configuration file. For example, the
-configuration of a filesystem storage with the code <cite>fsprod</cite> can be provided in the
-main configuration file as follows:</p>
-<pre class="code ini literal-block">
-<span class="k">[fs_storage.fsprod]</span><span class="w">
-</span><span class="na">protocol</span><span class="o">=</span><span class="s">s3</span><span class="w">
-</span><span class="na">options={&quot;endpoint_url&quot;</span><span class="o">:</span><span class="w"> </span><span class="s">&quot;https://my_s3_server/&quot;</span><span class="na">, &quot;key&quot;</span><span class="o">:</span><span class="w"> </span><span class="s">&quot;KEY&quot;</span><span class="na">, &quot;secret&quot;</span><span class="o">:</span><span class="w"> </span><span class="s">&quot;SECRET&quot;</span><span class="na">}</span><span class="w">
-</span><span class="na">directory_path</span><span class="o">=</span><span class="s">my_bucket</span>
-</pre>
-<p>To work, a <cite>storage.backend</cite> record must exist with the code <cite>fsprod</cite> into the database.
-In your configuration section, you can specify the value for the following fields:</p>
-<ul class="simple">
-<li><cite>protocol</cite></li>
-<li><cite>options</cite></li>
-<li><cite>directory_path</cite></li>
-</ul>
-</div>
 <div class="section" id="migration-from-storage-backend">
-<h3><a class="toc-backref" href="#toc-entry-4">Migration from storage_backend</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-3">Migration from storage_backend</a></h3>
 <p>The fs_storage addon can be used to replace the storage_backend addon. (It has
 been designed to be a drop-in replacement for the storage_backend addon). To
 ease the migration, the <cite>fs.storage</cite> model defines the high-level methods
@@ -552,7 +528,7 @@ <h3><a class="toc-backref" href="#toc-entry-4">Migration from storage_backend</a
 </div>
 </div>
 <div class="section" id="known-issues-roadmap">
-<h2><a class="toc-backref" href="#toc-entry-5">Known issues / Roadmap</a></h2>
+<h2><a class="toc-backref" href="#toc-entry-4">Known issues / Roadmap</a></h2>
 <ul class="simple">
 <li>Transactions: fsspec comes with a transactional mechanism that once started,
 gathers all the files created during the transaction, and if the transaction
@@ -566,30 +542,30 @@ <h2><a class="toc-backref" href="#toc-entry-5">Known issues / Roadmap</a></h2>
 </ul>
 </div>
 <div class="section" id="changelog">
-<h2><a class="toc-backref" href="#toc-entry-6">Changelog</a></h2>
+<h2><a class="toc-backref" href="#toc-entry-5">Changelog</a></h2>
 <div class="section" id="section-1">
-<h3><a class="toc-backref" href="#toc-entry-7">16.0.1.2.0 (2024-02-06)</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-6">16.0.1.2.0 (2024-02-06)</a></h3>
 <p><strong>Features</strong></p>
 <ul class="simple">
 <li>Invalidate FS filesystem object cache when the connection fails, forcing a reconnection. (<a class="reference external" href="https://github.com/OCA/storage/issues/320">#320</a>)</li>
 </ul>
 </div>
 <div class="section" id="section-2">
-<h3><a class="toc-backref" href="#toc-entry-8">16.0.1.1.0 (2023-12-22)</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-7">16.0.1.1.0 (2023-12-22)</a></h3>
 <p><strong>Features</strong></p>
 <ul class="simple">
 <li>Add parameter on storage backend to resolve protocol options values starting with $ from environment variables (<a class="reference external" href="https://github.com/OCA/storage/issues/303">#303</a>)</li>
 </ul>
 </div>
 <div class="section" id="section-3">
-<h3><a class="toc-backref" href="#toc-entry-9">16.0.1.0.3 (2023-10-17)</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-8">16.0.1.0.3 (2023-10-17)</a></h3>
 <p><strong>Bugfixes</strong></p>
 <ul class="simple">
 <li>Fix access to technical models to be able to upload attachments for users with basic access (<a class="reference external" href="https://github.com/OCA/storage/issues/289">#289</a>)</li>
 </ul>
 </div>
 <div class="section" id="section-4">
-<h3><a class="toc-backref" href="#toc-entry-10">16.0.1.0.2 (2023-10-09)</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-9">16.0.1.0.2 (2023-10-09)</a></h3>
 <p><strong>Bugfixes</strong></p>
 <ul class="simple">
 <li>Avoid config error when using the webdav protocol. The auth option is expected
@@ -600,30 +576,30 @@ <h3><a class="toc-backref" href="#toc-entry-10">16.0.1.0.2 (2023-10-09)</a></h3>
 </div>
 </div>
 <div class="section" id="bug-tracker">
-<h2><a class="toc-backref" href="#toc-entry-11">Bug Tracker</a></h2>
+<h2><a class="toc-backref" href="#toc-entry-10">Bug Tracker</a></h2>
 <p>Bugs are tracked on <a class="reference external" href="https://github.com/OCA/storage/issues">GitHub Issues</a>.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us to smash it by providing a detailed and welcomed
 <a class="reference external" href="https://github.com/OCA/storage/issues/new?body=module:%20fs_storage%0Aversion:%2016.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**">feedback</a>.</p>
 <p>Do not contact contributors directly about support or help with technical issues.</p>
 </div>
 <div class="section" id="credits">
-<h2><a class="toc-backref" href="#toc-entry-12">Credits</a></h2>
+<h2><a class="toc-backref" href="#toc-entry-11">Credits</a></h2>
 <div class="section" id="authors">
-<h3><a class="toc-backref" href="#toc-entry-13">Authors</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-12">Authors</a></h3>
 <ul class="simple">
 <li>ACSONE SA/NV</li>
 </ul>
 </div>
 <div class="section" id="contributors">
-<h3><a class="toc-backref" href="#toc-entry-14">Contributors</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-13">Contributors</a></h3>
 <ul class="simple">
 <li>Laurent Mignon &lt;<a class="reference external" href="mailto:laurent.mignon&#64;acsone.eu">laurent.mignon&#64;acsone.eu</a>&gt;</li>
 <li>Sébastien BEAU &lt;<a class="reference external" href="mailto:sebastien.beau&#64;akretion.com">sebastien.beau&#64;akretion.com</a>&gt;</li>
 </ul>
 </div>
 <div class="section" id="maintainers">
-<h3><a class="toc-backref" href="#toc-entry-15">Maintainers</a></h3>
+<h3><a class="toc-backref" href="#toc-entry-14">Maintainers</a></h3>
 <p>This module is maintained by the OCA.</p>
 <a class="reference external image-reference" href="https://odoo-community.org">
 <img alt="Odoo Community Association" src="https://odoo-community.org/logo.png" />

fs_storage/tests/test_fs_storage.py

@@ -1,7 +1,6 @@
 # Copyright 2023 ACSONE SA/NV (http://acsone.eu).
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
 import warnings
-from unittest import mock
 
 from odoo.exceptions import ValidationError
 from odoo.tests import Form
@@ -70,7 +69,12 @@ def test_ensure_one_fs_by_record(self):
         for i in range(4):
             backend_ids.append(
                 self.backend.create(
-                    {"name": f"name{i}", "directory_path": f"{i}", "code": f"code{i}"}
+                    {
+                        "name": f"name{i}",
+                        "directory_path": f"{i}",
+                        "code": f"code{i}",
+                        "protocol": "odoofs",
+                    }
                 ).id
             )
         records = self.backend.browse(backend_ids)
@@ -133,25 +137,6 @@ def test_interface_values(self):
         # this is still true after saving
         self.assertEqual(new_storage.options_protocol, protocol)
 
-    def test_options_env(self):
-        self.backend.json_options = {"key": {"sub_key": "$KEY_VAR"}}
-        eval_json_options = {"key": {"sub_key": "TEST"}}
-        options = self.backend._get_fs_options()
-        self.assertDictEqual(options, self.backend.json_options)
-        self.backend.eval_options_from_env = True
-        with mock.patch.dict("os.environ", {"KEY_VAR": "TEST"}):
-            options = self.backend._get_fs_options()
-            self.assertDictEqual(options, eval_json_options)
-        with self.assertLogs(level="WARNING") as log:
-            options = self.backend._get_fs_options()
-        self.assertIn(
-            (
-                f"Environment variable KEY_VAR is not set for "
-                f"fs_storage {self.backend.display_name}."
-            ),
-            log.output[0],
-        )
-
     def test_no_create_in_safe_eval(self):
         # check that we can't create a file in safe_eval
         with self.assertRaisesRegex(