[PUP-7314] make import of named elements into a name space work as an alias Created: 2017/03/07  Updated: 2019/02/21  Resolved: 2019/02/21

Status: Closed
Project: Puppet
Component/s: Language
Affects Version/s: None
Fix Version/s: None

Type: New Feature Priority: Normal
Reporter: Rick Sherman Assignee: Henrik Lindberg
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Cloners
is cloned by PUP-7634 Puppet device - make it threadable Closed
Support
supports PUP-1829 Define PCore - the Puppet Meta Meta M... Developing
Template:
Sub-team: Language
Team: Server
Sprint: Language Triage
QA Risk Assessment: Needs Assessment

 Description   

Make it possible to import names from a namespace to local names.

(TODO: Copy text from comments showing the idea to here).

This was originally reported as a desire to shorten names when using Pcore.

ORIGINAL


We would like to have PCore objects load their module level namespace implicitly.

This functionality exists with the init_typeset, but no other typesets.

This would be used in manifests and the output of `puppet resource`

resource { 'name':
  pcore_attr => Typeset::Type({
  'attr' => 'value',
  }),
}

vs

resource { 'name':
  pcore_attr => Module::Typeset::Type({
  'attr' => 'value',
  }),
}



 Comments   
Comment by Henrik Lindberg [ 2017/03/07 ]

The functionality in the init type set is because there you are referencing other things in the same type set - within the typeset all the references are relative to the typeset itself - this because you do not know inside of the typeset where it may be "mounted" into a puppet name space.

The use of types in the example here are general "external" references to types.

Comment by Henrik Lindberg [ 2017/03/07 ]

I think we need an explicit mechanism to shorten the names. If we instead try to do relative namespace resolution we go back to the time when puppet did have this - and that never worked well and created all kinds of hard to find problems.

For an explicit solution we could potentially use the (now defunct) keyword import (which was used for import of manifests). We could repurpose that keyword to allow names/aliases to be introduced at file scope.

The above (making it possible to refer to MyModule::Typeset as just Typeset) would then be expressed like this

import MyModule::Typeset TypeSet

or

import TypeSet = MyModule::TypeSet

The import would be different from a type alias as it would only apply to the file it appears in.

Comment by Thomas Hallgren [ 2017/03/07 ]

That solution still requires that you prefix everything with the name used for the import, i.e. no gain at all compared to the init_typeset in a module.

Another idea would be to have an explicit mechanism that shortens all names (not just names of types). Example:

namespace MyModule {
  // names in here are first resolved locally as <name>, then globally as MyModule::<name>, and lastly as ::<name>
}

It should be possible to nest such namespaces (similar to Ruby modules).

Comment by Henrik Lindberg [ 2017/03/07 ]

That would require a new keyword which is a bigger thing and would need to wait until major release.
I am also somewhat reluctant over what nesting of namespaces would mean as it kind of suggests containment rather than lexical scoping.

If the intent is to make all the names available I can imagine something like:

import MyModule::TypeSet

or some variation thereof - i.e. make all the names in that namespace be names in this file. That as an alternative to mounting them on a prefix - e.g.

import MyPrefix = MyModule::TypeSet

Comment by Thomas Hallgren [ 2017/03/08 ]

I guess the imports should then be limited to types and typesets? Or should we allow import of other names as well? Functions?

Comment by Henrik Lindberg [ 2017/03/08 ]

the immediate need is for data types - I can imagine importing other things as well (probably not right away). We could use more keywords as I think it could be good to keep names apart. As an example for functions it could be:

import function ModuleName
import function ModuleName::namespace
import function ModuleName::namespace::namespace::function
import function myname = ModuleName::namespace
# etc...

(without too much thought going into the different use cases).

Comment by Thomas Hallgren [ 2017/03/08 ]

Since data types will be the, probably by far, most common use-case, perhaps it would be good to then decide that the default, without a keyword, is to import a type namespace. I.e.

import MyModule # this imports all types in MyModule
import type MyModule # this also imports types in MyModule

Comment by Henrik Lindberg [ 2017/03/08 ]

yes, agree

Comment by Henrik Lindberg [ 2017/03/08 ]

Thought about the possible rules here. I think that if you import a typeset, all of the types in it are imported to their leaf name. If you fully specify a type, it is imported with its leaf name. If you use an assignment and the RHS is a typeset, then you alias the typeset, if it is a (leaf) type, then it is an alias for that type - the same way as if you had written type X = A::B::C (but with a file local scope for X).

When you import, the question is how shadowing should be handled. Strange things will happen if you for instance import the name Integer. I suppose it is best if we enforce that shadowing is not allowed - you would then be forced to use an import of the typeset by giving a short local name (i.e. use something like import X = MyModule::A::B). Don't remember what we allow in the 4.x function API for local types - can they shadow?

Does that make sense?

Comment by Thomas Hallgren [ 2017/03/08 ]

Yes, and no. All types in a typeset can be auto-loaded without knowledge of if they live in a typeset or not. The typeset is hence, just a packaging detail for types that are available to the auto-loader. With that in mind, perhaps typesets should be taken out of the equation altogether. I.e. you import a namespace, not a typeset.

Comment by Henrik Lindberg [ 2017/03/08 ]

In one way that is nice (namespace) - kind of like '*' in java, OTOH, the '*' in java can create problems (and the reason why IDEs expand the '*' to the classes you actually used and then saves that. Not sure what I like best.

With reference to typeset you need to know that the set exist as opposed to its individual types. Makes refactoring harder (create a set out of types). Am I missing something else we should consider?

Comment by Thomas Hallgren [ 2017/03/08 ]

Don't think so.
The way I envision it (and currently experimenting with) is a loader that is initialized with a set of imports. Each import is a TypedName (which means that we, in the future, also can allow import to denote a name authority). The loader is parented by the loader that would otherwise be in effect for the given file.

When resolving, the loader will prefix any name passed to it with each name from its import list and pass it to its parent loader. That processing ends as soon as the parent loader finds something, or if nothing is found, by passing the original name verbatim to the parent loader.

Comment by Henrik Lindberg [ 2017/03/09 ]

That sounds like it could be very slow. I ask for X an I have made 10 imports - the system now has to attempt 10 searches.
If the loader instead read a typeset, or referred to just a single type it can have an index of all imported names - it then creates the resolved name and asks the parent to load it. The loader only needs to read the typeset to build the index.

Comment by Thomas Hallgren [ 2017/03/09 ]

I think you oversimplify the typeset case. The auto-loader would not allow a typeset to simply shadow parent loaders. Nor will it say that a typeset shadows a type explicitly defined in a sub-directory in the same module. So, while perhaps slow (although, personally, I don't think that will be a big problem in the scheme of things), I don't think optimizing it the way you suggest is feasible.

Comment by Rick Sherman [ 2017/03/09 ]

Added Thomas Honey to the watch list so he will also put eyes on this.

In fear of derailing the current conversation, I will comment that this also makes me wonder if we can solve another one of our problems in this work.

Currently we are generating individual PCore objects for each YANG "object" (container), and have run into duplicate names

resource { 'name':
  first => Typeset::Type({
  'attr' => Typeset::Duplicate({
    'attr' = "stuff" }),
  }),
  second => Typeset::Other({
  'attr' => Typeset::Duplicate({
    'iama' = "different",
    'object' = "entirely" }),
  }),
}

May we be able to search for things throughout the hierarchy to find nested types, so that we can have these simple duplicated names? (I'm fully anticipate this will require more conversation to explain).

Comment by Henrik Lindberg [ 2017/03/10 ]

Rick Sherman yes, that needs more information regarding why you end up with the duplicate type names.

Comment by Rick Sherman [ 2017/03/10 ]

The YANG models themselves have duplication. For example this particular model point has 3 instances of "address":

             |  +--rw (address-choice)?
             |  |  +--:(address)
             |  |     +--rw address
             |  |        +--rw (address-choice)?
             |  |           +--:(fixed-case)
             |  |           |  +--rw primary
             |  |           |  |  +--rw address    inet:ipv4-address
             |  |           |  |  +--rw mask       inet:ipv4-address
             |  |           |  +--rw secondary* [address]
             |  |           |     +--rw address      inet:ipv4-address
             |  |           |     +--rw mask         inet:ipv4-address
             |  |           |     +--rw secondary?   empty
             |  |           +--:(dhcp-case)
             |  |              +--rw dhcp!
 
             +--rw ipv6
             |  +--rw address
             |  |  +--rw (address-choice)?
             |  |     +--:(autoconfig-case)
             |  |     |  +--rw autoconfig!
             |  |     |     +--rw default?   empty
             |  |     +--:(manual-case)
             |  |     |  +--rw prefix-list* [prefix]
             |  |     |     +--rw prefix     ios:ipv6-prefix
             |  |     |     +--rw anycast?   empty
             |  |     |     +--rw eui-64?    empty
             |  |     +--:(link-local-case)
             |  |        +--rw link-local-address* [address]
             |  |           +--rw address       inet:ipv6-address
             |  |           +--rw link-local?   empty
 
             +--rw peer
             |  +--rw default
             |     +--rw ip
             |        +--rw address
             |           +--rw (address-choice)?
             |              +--:(dhcp)
             |              |  +--rw dhcp?        empty
             |              +--:(dhcp-pool)
             |              |  +--rw dhcp-pool!
             |              |     +--rw pools?   string
             |              +--:(pool)
             |                 +--rw pool!
             |                    +--rw pools?   string

Which results in us generating:

type Vanilla_ice::Ned_interface::Address = Object[{
  attributes => {
    primary => Optional[Vanilla_ice::Ned_interface::Primary],
    secondary => Optional[Array[Vanilla_ice::Ned_interface::Secondary]],
    dhcp => Optional[Vanilla_ice::Ned_interface::Dhcp],
    xml_mapping => {type => Hash[String,String], value => { '_puppet_property' => 'address' }, kind => constant},
    xmlns => {type => String, value => "http://cisco.com/ns/yang/ned/ios", kind => constant},
}}]
 
 
type Vanilla_ice::Ned_interface::Address = Object[{
  attributes => {
    autoconfig => Optional[Vanilla_ice::Ned_interface::Autoconfig],
    prefix_list => Optional[Array[Vanilla_ice::Ned_interface::Prefix_list]],
    link_local_address => Optional[Array[Vanilla_ice::Ned_interface::Link_local_address]],
    xml_mapping => {type => Hash[String,String], value => { '_puppet_property' => 'address', 'prefix_list' => 'prefix-list', 'link_local_address' => 'link-local-address' }, kind => constant},
    xmlns => {type => String, value => "http://cisco.com/ns/yang/ned/ios", kind => constant},
}}]
 
 
type Vanilla_ice::Ned_interface::Address = Object[{
  attributes => {
    dhcp => Optional[Vanilla_ice::YangEmpty],
    dhcp_pool => Optional[Vanilla_ice::Ned_interface::Dhcp_pool],
    pool => Optional[Vanilla_ice::Ned_interface::Pool],
    xml_mapping => {type => Hash[String,String], value => { 'dhcp_pool' => 'dhcp-pool', '_puppet_property' => 'address' }, kind => constant},
    xmlns => {type => String, value => "http://cisco.com/ns/yang/ned/ios", kind => constant},
}}]

Comment by Henrik Lindberg [ 2017/03/14 ]

Rick Sherman That seems to require using the much longer yang name paths to make the types unique.
Were you thinking that + imports/aliasing would make it easier to use those longer names ?

Comment by Henrik Lindberg [ 2017/03/16 ]

After a long talk with Rick Sherman we came to the conclusion that the generated types must have fully qualified names as they are indeed different entities. The UX problem is that we require the use of the fully qualified names. The suggested import will help with shortening the names, but cannot shorten them beyond the most common root used in a .pp file - which still requires quite long names to be used.

Shortening the names further is a separate issue - the import is still of value.

Comment by Thomas Honey [ 2017/03/20 ]

I think that we are seeing an edge case of the problem.
The paths are long because on the number of nested structures, this is how the vendors have chosen to model the problem.
It is acceptable that the import path is long. in my opinion, we only reflect the underlying yang model.

Comment by Henrik Lindberg [ 2017/05/31 ]

I think Thomas Honey is saying that we can close this issue ?? Or rather, we can take it over to be the "import" issue for Pcore (instead of being a Yang specific ticket).

Comment by Henrik Lindberg [ 2019/02/21 ]

After continued discussion decided this is probably a bad idea - a similar feature in go-lang has for instance been slated for removal. Closing as won't do.

Generated at Mon Dec 09 02:14:42 PST 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.