idea: make "deprecated" marker into script #87

New Issue

ungleich-gitea · 2021-11-20T13:23:42Z

ungleich-gitea commented

2021-11-20 13:23:42 +00:00

i want to deprecate some parameters in type and want to display warning or error only if those parameters are in use.

so, I suggest following:

deprecated follows same logic as gencode scripts - if x bit is set, then execute, else run with sh -e.

deprecated will run after type explorers.

if output is produced, use that in WARNING log and continue working.

if deprecated script terminates with exit 1, then terminate cdist with ERROR and display output.

usecase:

for upcoming changes in __acl I want to replace some parameters, but maybe we want to have compatibility mode (old paremeters are translated into new ones) for some time, hence just warning. and with next major version we remove compatibility and produce ERROR if removed parameters are used. and after some time remove deprecated script.

or maybe that's too much complexity and just make breaking changes with next major version?

I personally don't like to maintain legacy compatibility, but if we think that's important, then we need to improved deprecation management.

i want to deprecate some parameters in type and want to display warning or error only if those parameters are in use. so, I suggest following: `deprecated` follows same logic as gencode scripts - if x bit is set, then execute, else run with `sh -e`. `deprecated` will run after type explorers. if output is produced, use that in WARNING log and continue working. if `deprecated` script terminates with `exit 1`, then terminate cdist with ERROR and display output. usecase: for upcoming changes in `__acl` I want to replace some parameters, but maybe we want to have compatibility mode (old paremeters are translated into new ones) for some time, hence just warning. and with next major version we remove compatibility and produce ERROR if removed parameters are used. and after some time remove `deprecated` script. or maybe that's too much complexity and just make breaking changes with next major version? I personally don't like to maintain legacy compatibility, but if we think that's important, then we need to improved deprecation management.

ungleich-gitea closed this issue

2021-11-20 13:23:42 +00:00

ungleich-gitea commented

2021-11-20 15:36:58 +00:00

closed

ungleich-gitea commented

2021-11-20 15:36:59 +00:00

@ander Closing this.
https://code.ungleich.ch/ungleich-public/cdist/merge_requests/786

@ander Closing this. https://code.ungleich.ch/ungleich-public/cdist/merge_requests/786

ungleich-gitea commented

2021-11-20 15:37:00 +00:00

mentioned in merge request !786

ungleich-gitea commented

2021-11-20 15:37:02 +00:00

Details should go into man page anyway, righy?

yes. just like I updated man for __acl

> Details should go into man page anyway, righy? yes. just like I updated man for __acl

ungleich-gitea commented

2021-11-20 15:37:03 +00:00

@ander yes, I meant on something like deprecated file containing list of deprecated params. First I meant to include such file for each param with message for each deprecated param, but one file with list of all deprecated params seems k.i.s.s. solution :D Details should go into man page anyway, righy?

ungleich-gitea commented

2021-11-20 15:37:04 +00:00

If it benefits most of our users

yes, it will benefit all 3 users! 😈

I added compatiblity for old parameters, but now we still have open question how do display warning only if deprecated parameters are used. @poljakowski whichever solution is easiest to implement, I'll go for it.

if parameter x is used then see if deprecated-x is present, and if so then WARN with deprecation message, similar to already supported type deprecation.

basically prefix all deprecated parameters with deprecated- and they still work (for gencode s/^deprecated-//)? sounds like good place for errors.

maybe just introduce parameter/deprecated which contains all deprecated parameters and still have to define deprecated parameters in parameter/optional too.

> If it benefits most of our users yes, it will benefit all 3 users! :smiling\_imp: I added compatiblity for old parameters, but now we still have open question how do display warning only if deprecated parameters are used. @poljakowski whichever solution is easiest to implement, I'll go for it. > if parameter `x` is used then see if `deprecated-x` is present, and if so then WARN with deprecation message, similar to already supported type deprecation. basically prefix all deprecated parameters with `deprecated-` and they still work (for gencode `s/^deprecated-//`)? sounds like good place for errors. maybe just introduce `parameter/deprecated` which contains all deprecated parameters and still have to define deprecated parameters in `parameter/optional` too.

ungleich-gitea commented

2021-11-20 15:37:06 +00:00

@ander How about adding new options and documenting old ones as present for backward compatibility? Or does this require additional complexity in type implementation?

ungleich-gitea commented

2021-11-20 15:37:07 +00:00

There is a general rule for this in cdist:

In general, we try not to break the user
** More often than obvious at first sight, we can avoid breaking the user (see below)
If we decide to break the user, we should have a good reason for it
If we plan to break something, we need visible output to the user for the deprecation message(s)
If something breaks, we need a new major version
** So usual approach: we have cdist X.Y.Z
** We plan to break something, so we create cdist X2.Y2.Z2 that has deprecation messages (where at least one of X2, Y2, Z2 is different from the previous version)
** The earliest time to break is then at cdist X+1.?.?
** So far we waited for cdist X+2 to break. Reasoning: X++ happens rarely and thus X+1 gives the users a good chance of adapting until we really break in X+2

When to break the user

If it remove significant maintenance burden
If it introduces significant new features
If it benefits most of our users

When not to break the user

If start to dislike a type
** Just create __$othername

HTH, greetings from the S25

There is a general rule for this in cdist: * In general, we try not to break the user ** More often than obvious at first sight, we can avoid breaking the user (see below) * If we decide to break the user, we should have a good reason for it * If we plan to break something, we need visible output to the user for the deprecation message(s) * If something breaks, we need a new major version ** So usual approach: we have cdist X.Y.Z ** We plan to break something, so we create cdist X2.Y2.Z2 that has deprecation messages (where at least one of X2, Y2, Z2 is different from the previous version) ** The earliest time to break is then at cdist X+1.?.? ** So far we waited for cdist X+2 to break. Reasoning: X++ happens rarely and thus X+1 gives the users a good chance of adapting until we really break in X+2 When to break the user * If it remove significant maintenance burden * If it introduces significant new features * If it benefits most of our users When not to break the user * If start to dislike a type ** Just create __$othername * <insert good reason here> HTH, greetings from the S25

ungleich-gitea commented

2021-11-20 15:37:08 +00:00

so, basically it's a question about how much legacy support we want to do.

@nico what's your take on this? also see first comment @ !785

so, basically it's a question about how much legacy support we want to do. @nico what's your take on this? also see first comment @ !785

ungleich-gitea commented

2021-11-20 15:37:10 +00:00

imho that's too much.

i could just leave old parameters intact, create type explorer which terminates if old explorers are used and call it a day.

imho that's too much. i could just leave old parameters intact, create type explorer which terminates if old explorers are used and call it a day.

ungleich-gitea commented

2021-11-20 15:37:11 +00:00

How about trying something like the following.
Since type parameters are handled by python core then in that code
we could see how easy/hard it is to support parameter deprecation.
E.g. if parameter x is used then see if deprecated-x is present,
and if so then WARN with deprecation message, similar to already supported
type deprecation.

What do you think?

How about trying something like the following. Since type parameters are handled by python core then in that code we could see how easy/hard it is to support parameter deprecation. E.g. if parameter `x` is used then see if `deprecated-x` is present, and if so then WARN with deprecation message, similar to already supported type deprecation. What do you think?

ungleich-gitea commented

2021-11-20 15:37:12 +00:00

mentioned in merge request !785