Skip to content

Conversation

@yankinmax
Copy link
Contributor

No description provided.

@yankinmax yankinmax mentioned this pull request Oct 24, 2025
1 task
@yankinmax yankinmax force-pushed the 18.0-mig-connector_importer_source_sftp branch from ae11221 to 4c59328 Compare October 24, 2025 11:01
@yankinmax yankinmax marked this pull request as draft October 27, 2025 08:02
@yankinmax
Copy link
Contributor Author

Hello @simahawk @sebalix @SilvioC2C can you pls help with any suggestions?
I'm not familiar with the module usage.
My debug session shows that filelist = self.list(relative_path) is empty, so no files are really found.
https://github.com/OCA/storage/blob/5bbfe12c01e68e562922838aa5fd0eb28dc235a0/storage_backend/components/base_adapter.py#L32

@yankinmax
Copy link
Contributor Author

yankinmax commented Oct 30, 2025

Hello @simahawk @sebalix @SilvioC2C can you pls help with any suggestions? I'm not familiar with the module usage. My debug session shows that filelist = self.list(relative_path) is empty, so no files are really found. https://github.com/OCA/storage/blob/5bbfe12c01e68e562922838aa5fd0eb28dc235a0/storage_backend/components/base_adapter.py#L32

So, the issue is definitely with the _selection_source_ref_id override to include import.source.csv.sftp source.
It's not called somehow 😢

https://github.com/OCA/connector-interfaces/pull/172/files#r2476825551

Comment on lines +14 to +19
def _selection_source_ref_id(self):
selection = super()._selection_source_ref_id()
new_source = ("import.source.csv.sftp", "CSV SFTP")
if new_source not in selection:
selection.append(new_source)
return selection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not working as expected

@@ -1,2 +1,2 @@
from . import source_csv_sftp
from . import source_mixin
from . import source_csv_sftp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivantodorovich this idea didn't help


_name = "import.source.csv.sftp"
_inherit = [
"import.source.csv",
Copy link
Contributor

Choose a reason for hiding this comment

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

try also inheriting from the mixin again here

Suggested change
"import.source.csv",
"import.source.csv",
"import.source.consumer.mixin",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivantodorovich something really unexpected happening here. Seems still not working as expected. I wonder if we could adapt to use fields.Selection here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, simplify if needed.

Copy link
Contributor Author

@yankinmax yankinmax Nov 4, 2025

Choose a reason for hiding this comment

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

Making source_model selection attribute as usual list of tuples and adding to the inherited class selection_add also doesn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simahawk @sebalix do you have any idea of the issue that _selection_source_ref_id override doesn't actually add new_source, simply not called.

connector_importer_source_sftp/models/source_mixin.py

@api.model
def _selection_source_ref_id(self):
    selection = super()._selection_source_ref_id()
    new_source = ("import.source.csv.sftp", "CSV SFTP")
    if new_source not in selection:
        selection.append(new_source)
    return selection

@yankinmax yankinmax force-pushed the 18.0-mig-connector_importer_source_sftp branch from 982071a to 902dfd5 Compare November 3, 2025 18:07
@ivantodorovich ivantodorovich force-pushed the 18.0-mig-connector_importer_source_sftp branch from 902dfd5 to 7ee25ea Compare November 6, 2025 12:58
@ivantodorovich ivantodorovich force-pushed the 18.0-mig-connector_importer_source_sftp branch 2 times, most recently from f63f2f0 to 6db6bd8 Compare November 6, 2025 14:35
@yankinmax yankinmax marked this pull request as ready for review November 6, 2025 15:51
@yankinmax
Copy link
Contributor Author

Thanks for help @ivantodorovich 🙏

Traceback:
```
  File ".../connector_importer_source_sftp/components/event_listeners.py", line 32, in _skip_move_file
    source = importer.recordset.get_source()
  File ".../connector_importer/models/sources/source_consumer_mixin.py", line 71, in get_source
    return self.source_ref_id
  File "/opt/odoo/odoo/fields.py", line 1303, in __get__
    self.compute_value(recs)
  File "/opt/odoo/odoo/fields.py", line 1485, in compute_value
    records._compute_field_value(self)
  File "/opt/odoo/odoo/models.py", line 5297, in _compute_field_value
    fields.determine(field.compute, self)
  File "/opt/odoo/odoo/fields.py", line 110, in determine
    return needle(*args)
  File ".../connector_importer/models/sources/source_consumer_mixin.py", line 43, in _compute_source_ref_id
    item.source_ref_id = f"{item.source_model},{item.source_id}"
  File "/opt/odoo/odoo/fields.py", line 1398, in __set__
    self.write(protected_records, value)
  File "/opt/odoo/odoo/fields.py", line 1205, in write
    cache_value = self.convert_to_cache(value, records)
  File "/opt/odoo/odoo/fields.py", line 3041, in convert_to_cache
    raise ValueError("Wrong value for %s: %r" % (self, value))
ValueError: Wrong value for import.recordset.source_ref_id: 'import.source.csv.sftp,1'
```

For some reason I don't know, by the time the event listener skip_if condition is
evaluated, the `import.recordset` model does not have applied the override to
`_selection_source_ref_id` in the mixin, and so the Reference field fails with
this ValueError.

The workaround here makes `get_source` not actually use the Reference field, but
directly `source_model` and `source_id`, which of course do not attempt to
validate the value.
… tests

For some reason, probably due to server_environment, if tests are ran during the
module installation, like in the oca-ci, the storage backend_type is
"filesystem", when it should be "sftp" according to the demo data.

Did not investigate any further.
@ivantodorovich ivantodorovich force-pushed the 18.0-mig-connector_importer_source_sftp branch from 6db6bd8 to 91c1be3 Compare November 19, 2025 15:43
@ivantodorovich
Copy link
Contributor

@simahawk this one's ready too

@simahawk
Copy link
Contributor

simahawk commented Dec 2, 2025

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-172-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 0724592 into OCA:18.0 Dec 2, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 947f1ac. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants