Addressed in https://github.com/vyos/vyos.vyos/pull/246

Let’s get tests at least for the changes here before we check it in. It should be trivial to test, especially since it looks like there is no version difference to take into account.

On it, @syncer

Started to look into this - but it seems to be based on older version of collection (57 commits behind) and failing all the sanity checks.
I will try and refresh that branch and re-test

Or I may just re-introduce the changes into the working current branch given they are all straight forward

Yeah, changes are pretty small and also pretty isolated. I’m guessing the merge might be fast, unless there are significant conflicts. However if there are conflicts probably faster to re-apply the change.

Sanity tests are failing pretty much all:
FATAL: The 5 sanity test(s) listed below (out of 34) failed. See error output above for details.
action-plugin-docs
import --python 3.12
pep8
pylint
validate-modules

Errors are similar to
this
ERROR: tests/unit/modules/network/vyos/test_vyos_bgp_address_family.py:57:4: arguments-renamed: Parameter 'filename' has been renamed to 'transport' in overriding 'TestVyosBgpafModule.load_fixtures' method
and
this
ERROR: plugins/modules/vyos_bgp_global.py:0:0: import-error: Exception attempting to import module for argument_spec introspection, 'No module named 'ansible_collections.ansible.netcommon.plugins.module_utils.network.common.resource_module''
ERROR: plugins/modules/vyos_facts.py:0:0: import-error: Exception attempting to import module for argument_spec introspection, 'No module named 'ansible_collections.ansible.netcommon.plugins.module_utils.network.common.network_template''

I cannot think this is my setup (as fork from official vyos.vyos does not fail) but some discrepancies due to being too far behind

was that after rebasing it onto our current main branch or applying changes directly? Or something else?

I believe it was forked a while ago (in March this year) and never refreshed after that - so the fork (and PK) is using older version of the repo

And I cloned the fork to my laptop to test

Maybe ask the author to resubmit against the current?

sure, ok - I will do needful for it to be tests ready
It does not appear a huge change to me

Update - I cloned the current vyo.vyos changes, created a branch and made the changes that Maxime proposed.

I had to update syntax here and there however sanity, unit tests were OK. I also successfully tested against v1.3.4

The code nonetheless is not compatible with 1.4+ as there is no 'firewall' setting in set interfaces command - compare https://docs.vyos.io/en/equuleus/configuration/firewall/index.html#applying-a-rule-set-to-an-interface and https://docs.vyos.io/en/stable/configuration/interfaces/ethernet.html

The branch I tested with is local - I cannot reuse the fork until I finish ntp change. Happy to hand over the local changes and the code

I'm back home now, feel free to send them to me and I'll take a look at it today (I have a few minutes now, but likely will need to look at it in about 12 hours).

I can finish those out tonight or tomorrow. (he says optimistically)