On 25 October 2016 at 18:23, Alex Schultz <[email protected]> wrote:
> On Tue, Oct 25, 2016 at 3:13 AM, David Schmitt <[email protected]> > wrote: > > > > > > On 24 October 2016 at 17:32, <[email protected]> wrote: > >> > >> > >> > >> On Friday, October 21, 2016 at 12:48:44 PM UTC-6, David Schmitt wrote: > >>> > >>> Hi, > >>> > >>> thank you for voicing your feedback. I can't do my work without it. > >>> > >>> On Thursday, October 20, 2016 at 10:49:45 AM UTC-7, [email protected] > >>> wrote: > >>>> > >>>> So I've recently noticed the deprecation notices for the validate_* > and > >>>> is_* functions withing stdlib. As a consumer of the stdlib who > currently > >>>> needs to continue to support puppet 3 and hasn't moved to puppet 4 > typing > >>>> for ~40 modules, this is a giant pain. > >>> > >>> > >>> If you are not yet prepared to make the switch, please stay with stdlib > >>> 4.12. > >>> > >>>> > >>>> Additionally we do not require (nor leverage) any of the old edge > cases > >>>> that are trying to continue to be maintained under the validate_legacy > >>>> function. > >>> > >>> > >>> validate_legacy and the Compat types are not supposed to continue to > >>> maintain the mess that were the validate_ functions. They are designed > to > >>> help you migrade in an incremental fashin, to leverage the new > datatypes, > >>> without forcing your complete installation to switch ot once into the > new > >>> world. If you have that kind of control over your modules, or you > already > >>> know that you're hitting none of the edge cases, you can of course > choose to > >>> do the switch in a single step. > >>> > >>>> > >>>> Is there a reason we can't just keep these is_* and validate_* > >>>> functions as is without the deprecation and/or just fix these in a > newer > >>>> version of stdlib? > >>> > >>> > >>> You can. Stay with stdlib 4.12. > >> > >> > >>> > >>> > >>>> > >>>> Is there some additional info as to why this decision was made? > >>> > >>> > >>> We want to start using the "new" puppet 4 features in the supported > >>> modules to show off the improvements you can gain through them. The > >>> deprecation and validate_legacy functions are intended to help the > whole > >>> ecosystem make this transition without having a flag day where > everyone has > >>> to switch. > >>> > >>> Using datatypes has a number of advantages over the validate functions: > >>> * high expressivity: look through the Compat types to see what the > >>> functions *actually* tested. They accept surprising types and leak > weird > >>> edge cases. Using datatypes removes a huge trap, and allows much > stricter > >>> specifications. > >>> * documentability: puppet-strings will surface datatypes in the > generated > >>> HTML. validate method calls are invisible. > >>> * core features: you can leverage the expressivity of datatypes using > the > >>> =~ match operator and assert_type everywhere you previously used > validate > >>> and is functoins, and the results have a much better chance of meeting > >>> everyone's expectations > >>> * extensiiblity: it is very easy to define custom types that match a > >>> module's domain, while it is very obscure to create your own validate > >>> functions. > >>> > >> > >> > >> Shouldn't these types of deprecation occur in a major version like in > the > >> 5.x series? > > > > > > Others already have answered this. > > > >> > >> I get the desire to move forward on these types of changes but the > >> problem I have is mostly with the forced (and silent) implementation of > >> these things mid 4.x. > > > > > > This is not mid-4.x. I expect this to be one of the very last 4.x > releases. > > There are a few bug fixes currently in flight around IP validation, but I > > see no reason to delay a stdlib 5.x release for much longer. > > > >> > >> Swapping out these changes mid 4.x series is not a very good transition > >> path for the end user. The problem I ran into while attempting to > address > >> these deprecations is that the validate_legacy does not exist until 4.13 > >> which would force our minimum required stdlib from the current >= 4.0.0 > < > >> 5.0.0 to >= 4.13.0 < 5.0.0. I also don't think the validate_legacy > works > >> under puppet 3. See > >> http://logs.openstack.org/71/389271/1/check/gate-puppet- > aodh-puppet-unit-3.8-centos-7/e89cc6b/console.html.gz#_2016- > 10-20_16_36_40_481087 > > > > > > To quote the stdlib readme on validate_legacy: "Note: This function > relies > > on internal APIs from Puppet 4.4.0 (PE 2016.1) onwards, and doesn't work > on > > earlier versions." > > Yes I see that now. I think this is a case where being giant, bold and > first would have been useful. > http://www.telegraph.co.uk/science/2016/10/23/internet- > is-becoming-unreadable-because-of-a-trend-towards-light/ > > > > >> The stdlib module is so ingrained in the community, I just think this > >> transition needs better thought around the impact to the end user. Just > >> pinning to <= 4.12.0 is not a quality answer because it just delays the > >> problem and can lead to incompatibilities between modules that continue > to > >> attempt to support both puppet 3 and 4. > > > > > > Only deployments (not modules) should pin to <= 4.12.0. This is a > decision > > each site needs to make on their own schedule. The upgrade path was > > specifically designed to avoid requiring coordination between modules. > > > >> > >> Puppet 3 is not EOL just yet and enterpise customers are always late > >> adopters so realistically these types of issues will only get larger > for the > >> foreseeable future. > > > > > > The whole stdlib/ntp/puppet4 feature transition is only useful/usable to > > people who are already on puppet4. Pulling the trigger on it now, while > > people are still under support and can be assisted with the arising > issues > > seemed much better than waiting until they drop out of support, and then > > pull the rug under their feet. > > > >> > >> Unfortunately for the puppet openstack modules which is what I'm > working > >> on specifically, we won't be officially dropping puppet 3 support until > >> after the current cycle which ends in March 2017 and we may need newer > >> version of stdlib. > > > > > > The deprecation function defaults to emitting no warnings under puppet3. > > Using validate_legacy - like all puppet 4 features - will break your > puppet3 > > users anyways. Which specific issues do you have using other stdlib > features > > on puppet 3? > > > >> This just seems like something that would be better suited for the next > >> major version than trying to do it mid stdlib 4.x and let people opt in > to > >> it as puppet 3 support fully dies off. > >> > >> Thanks, > >> -Alex > >> > >> p.s. I'm not sure that "if $var =~ Stdlib::Compat::Array" is nearly as > >> convenient (or readable) as if is_array($var) and trying to use standard > >> types instead of just validate_re is just painful. > > > > > > The ::Compat:: types are specifically designed for enabling the step out > of > > the validate_ functions, and should not be used in any other case. If you > > look at their implementation, you'll see the limitations of the legacy > > functions. After getting off the validate_ and is_ functions, you can use > > the plain core types like this: > > > > if $var =~ Array[String] { # or whatever element type you're expecting > > Yea I'll just agree to disagree on this one. It was more of the > syntax choice I was referring to. > You might prefer assert_type() then ? https://docs.puppet.com/puppet/latest/reference/function.html#asserttype > > > > Regular expressions are a first-class construct in puppet 4, so you don't > > need to use type validation at all: > > > > if $var =~ /^some re$/ { # > > https://docs.puppet.com/puppet/4.7/reference/lang_ > data_regexp.html#syntax > > > > > > Much better than hiding your type expectations in run-time checks is > putting > > the expected types into the class definition, where they can easily be > found > > by your callers, extracted into documentation by puppet-strings, and > > produces error messages at the call-site, and not within your module. > > > > I don't necessarily agree that is_array is hiding my type > expectations. I think that's dependent on how the end user is > consuming puppet. I think this is where something that is better for > developers does not necessarily translate or make the end user's life > better/easier. > Compare the test-output at https://davids.github.io/puppetlabs-ntp/puppet_classes/ntp.html to anything handwritten. puppet-strings is great. Everyone should use it. I just need to find a poor soul to move everything from the Reference section in the README into the code comments :-/ Anyway, thanks for taking the time to explain these issues. I think > much of my issue around this change was really expectations on how it > was conveyed. As an end user who may not be deeply entrenched in many > of these technical changes between 3/4 (as they are not strict > requirements for our use cases), some these items are not clear based > on what is presented. My first experience with this is a bunch of > unhelpful warnings and exceptions in my logs. In this case I think > there were a few things that would have been beneficial: > > 1) Include more details in the deprecation notice including a time > frame. For example, "The is_string function is deprecated and will be > removed in 5.x" It's simple to understand and as an end user provides > a time frame to which I need to respond. Additionally it includes the > function name that is deprecated. The message that is provided > "Warning: This method is deprecated, please use the stdlib > validate_legacy function, with Stdlib::Compat::String. There is > further documentation for validate_legacy function in the README." > provides some hinting as a way forward but it's not really clear to > the end user for the next steps or when this needs to be addressed. > The validate_legacy recommendation should probably live in README and > not the actual warning. This is where much of my confusion came from > as the validate_legacy is technically not a drop in replacement for me > due to the puppet 4.4 requirement. > > 2) Deprecation notices in the documentation should be *bold* and first > with a clear recommended alternative or proposed path forward. It > appears that none if the is_ or validate_ functions are marked as > deprecated in the documentation. > https://github.com/puppetlabs/puppetlabs-stdlib#is_absolute_path > > 3) I'd put the puppet 4.4 requirement first under validate_legacy > (also bold). I'm not sure this should be pushed forward as the first > alternative. It seems like it might be useful for transition periods > but it seems like it might be better to just switch unless there's a > really good reason to rely on historic edge cases. But that's just > how I've interpreted it after all of this. > Thank you again for your feedback and patience. I've created https://tickets.puppetlabs.com/browse/MODULES-4008 for the docs issues, and am currently working on fixing the wording of the deprecation warnings from the functions. D. > > Thanks, > -Alex > > > > > Regards, David > > > >>>> Having to go through our modules and switch out to the validate_legacy > >>>> functions is an effort we don't have the resources to undertake and > the > >>>> deprecation notices aren't something we can live with as they make it > very > >>>> hard to figure out when something actually breaks. > >>> > >>> > >>> Please see the documentation for the deprecation function in the stdlib > >>> readme on how to turn on/off deprecations in different situations (via > >>> puppet configuration on your master, or a environment variable during > >>> testing). You always have the possibility to stay on stdlib 4.12 until > you > >>> are ready to start your upgrade project. > >>> > >>>> > >>>> Additionally I'd like to point out that the deprecation notices make > it > >>>> next to impossible to figure out what is deprecated, see > >>>> http://logs.openstack.org/89/388589/1/gate/gate-puppet- > openstack-integration-4-scenario001-tempest-centos-7/ > fc2567b/console.html#_2016-10-19_22_24_59_667975 > >>>> > >>> > >>> Crap. I missed that one. I'm currently at puppetconf, and travelling > home > >>> afterwards, so I won't be able to look into it immediately, but I've > created > >>> https://tickets.puppetlabs.com/browse/MODULES-3993 to track this, and > will > >>> get to it next week. Until then, grepping for 'validate_|is_' is > probably a > >>> good first approximation of everything you'll need to address. > >>> > >>> > >>> Cheers, David > >> > >> -- > >> You received this message because you are subscribed to a topic in the > >> Google Groups "Puppet Developers" group. > >> To unsubscribe from this topic, visit > >> https://groups.google.com/d/topic/puppet-dev/ruPhY0Oks6A/unsubscribe. > >> To unsubscribe from this group and all its topics, send an email to > >> [email protected]. > >> To view this discussion on the web visit > >> https://groups.google.com/d/msgid/puppet-dev/7d10843f- > 4647-4e07-acea-95bd765431b4%40googlegroups.com. > >> For more options, visit https://groups.google.com/d/optout. > > > > > > -- > > You received this message because you are subscribed to a topic in the > > Google Groups "Puppet Developers" group. > > To unsubscribe from this topic, visit > > https://groups.google.com/d/topic/puppet-dev/ruPhY0Oks6A/unsubscribe. > > To unsubscribe from this group and all its topics, send an email to > > [email protected]. > > To view this discussion on the web visit > > https://groups.google.com/d/msgid/puppet-dev/CALF7fHbjSNS6tAcPyScMn% > 3DU66iK6AK22zQ444dv1zxJORFafcA%40mail.gmail.com. > > > > For more options, visit https://groups.google.com/d/optout. > > -- > You received this message because you are subscribed to a topic in the > Google Groups "Puppet Developers" group. > To unsubscribe from this topic, visit https://groups.google.com/d/ > topic/puppet-dev/ruPhY0Oks6A/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > [email protected]. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/puppet-dev/CAFsb3b6yA6FKc5f5%2Bsj0HCExadjbMgBUbvsauZ64JHGbd > Pev0w%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CALF7fHYKGaGussxtnAq6OK5cNJAWBk8M0Ny2G7SHBU6A-RCuEA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
