idea: make "deprecated" marker into script #87

Closed
opened 2021-11-20 13:23:42 +00:00 by ungleich-gitea · 13 comments

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.
Author
Owner

closed

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

mentioned in merge request !786

mentioned in merge request !786
Author
Owner

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
Author
Owner

@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?

@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?
Author
Owner

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.
Author
Owner

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

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

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
Author
Owner

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
Author
Owner

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.
Author
Owner

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?
Author
Owner

mentioned in merge request !785

mentioned in merge request !785
Author
Owner

also, there is probably only 2 more __acl users in the world besides me 😈

also, there is probably only 2 more `__acl` users in the world besides me :smiling\_imp:
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: ungleich-public/cdist#87
No description provided.