Change collection PS util import pattern #61307
Conversation
jborean93
requested review from
nitzmahone,
bcoca and
mattclay
and removed request for
bcoca
ansibot
added
affects_2.9
ansibot
removed
the
needs_triage
There was a problem hiding this comment.
Looks good- one thing to consider:
As a developer quality-of-life thing, it might be good to split the "find the #AnsibleRequires lines" away from the "load the contents" (not necessarily for freeze either, but as a bugfix/enhancement after), so that a malformed requires statement can give an error instead of being silently ignored because the regex didn't find it. It'd also probably make the regexes a little more "bite-size" and easier to follow.
I definitely like the "less magic" approach here to what I originally did, and the type accelerator thing is a good way to make that feature more accessible to stop the madness on verbosity of static/type refs.
| self._re_cs_module = [ | ||
| # Reference C# module_util in another C# util | ||
| re.compile(to_bytes(r'(?i)^using\s((Ansible\..+)|' | ||
| r'(ansible_collections\.[\w.]+\.[\w.]+\.plugins\.module_utils\.[\w.]+));\s*$')), |
There was a problem hiding this comment.
Why include the . in the character groups? [\w.] is the same as [.]- and allows lots of things we don't want...
There was a problem hiding this comment.
The original regex originally was \w.+\. which mean match 1 word char then allow any char until .. Happy to change this to be just \w+ to match [A-Za-z0-0_] but having the . in there before made me pause.
Sounds fair enough, I'll add a comment for future work.
I thought it was a good idea and potentially a nice way to mask a potentially namespace/class change of the builtin C# utils if we ever do that. My only misgiving is that it potentially allows someone to have a name conflict if they don't really use something unique. |
jborean93
force-pushed
the
ps-collection-util
branch
from
c60cb5a
to
808d005
Compare
|
CI failures are unrelated to changes. |
* Change collection PS util import pattern * Add changes for py2 compat * fix up regex and doc errors * fix up import analysis * Sanity fix for 2.6 CI workers * Get collection util path for coverage collection
* Change collection PS util import pattern * Add changes for py2 compat * fix up regex and doc errors * fix up import analysis * Sanity fix for 2.6 CI workers * Get collection util path for coverage collection
SUMMARY
Change the way PowerShell and C# module utils inside a collection are referenced. This is a breaking change but is being done to unify import patterns between the Python, PowerShell, and C#. Collections are a tech preview in 2.8 so this should hopefully be in by 2.9 before the existing pattern is more solidified.
This PR also adds a way to specify a type accelerator inside a C# util to simplify the type name a bit more. This is optional and a suggestion to add to Ansible but can have consequences if 2 utils use the same accelerator.
This also requires some changes in
ansible-testbut waiting to see what @nitzmahone and @mattclay says before delving in there.ISSUE TYPE
COMPONENT NAME
PowerShell
Collections