Seeding templates overrides custom changes - should we lock templates we ship?

Hello foreman-devs,

recently I was told about the bug that we override all templates in database
whenever we run db:seed. From the code [1] and commit message [2], it was not
the intended behavior. It was supposed to check whether user made some changes
and only apply the new version if the template was not touched. Sadly, the
method only checks the name attribute for changes [3], so if "only" template
content was changed, we still override it.

While I can try to fix it to originally intended behavior, I'd like to ask
whether it wouldn't be better to use this opportunity and start locking
templates we ship by default. The recommended workflow for users would be to
clone the template if custom changes are needed. We'd always update locked
templates. Obviously, user would need to merge new version to cloned template
on his own. With foreman_templates plugin it should be easy enough to export
templates and see the diff between default and customized template, apply the
changes user wants and then reimport them back.

I think this would be overall better user experience and safer workflow. The
originally intended behavior would never update a template that user modified.
That means after update user ends up with template from old Foreman version
(with custom changes) that might not be compatible with the new Foreman
version. This is more likely to happen than before because we now version
templates in community-repo and we don't keep backward compatibility as we did
before.

Thanks for reading, thoughts?

[1] https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provisioning_templates.rb#L98
[2] https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563
[3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33

··· -- Marek

+1 to locking

I have messed up a production install by changing a template for personal
use, not realizing that they span other orgs… oops!

··· On Fri, Feb 17, 2017 at 9:04 AM, Marek Hulán wrote:

Hello foreman-devs,

recently I was told about the bug that we override all templates in
database
whenever we run db:seed. From the code [1] and commit message [2], it was
not
the intended behavior. It was supposed to check whether user made some
changes
and only apply the new version if the template was not touched. Sadly, the
method only checks the name attribute for changes [3], so if "only"
template
content was changed, we still override it.

While I can try to fix it to originally intended behavior, I’d like to ask
whether it wouldn’t be better to use this opportunity and start locking
templates we ship by default. The recommended workflow for users would be
to
clone the template if custom changes are needed. We’d always update locked
templates. Obviously, user would need to merge new version to cloned
template
on his own. With foreman_templates plugin it should be easy enough to
export
templates and see the diff between default and customized template, apply
the
changes user wants and then reimport them back.

I think this would be overall better user experience and safer workflow.
The
originally intended behavior would never update a template that user
modified.
That means after update user ends up with template from old Foreman version
(with custom changes) that might not be compatible with the new Foreman
version. This is more likely to happen than before because we now version
templates in community-repo and we don’t keep backward compatibility as we
did
before.

Thanks for reading, thoughts?

[1] https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.
d/07-provisioning_templates.rb#L98
[2] https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563
[3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33


Marek


You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

As a heavy and active user, I'd fully support locking. There are quite
often new features / functionalities / bugfixes / new OS support etc in the
new templates. Reading these differences and utilising these as a reference
for our custom templates is a good way to go imho.

··· On Friday, February 17, 2017 at 9:05:13 AM UTC-5, Marek Hulán wrote: > > Hello foreman-devs, > > recently I was told about the bug that we override all templates in > database > whenever we run db:seed. From the code [1] and commit message [2], it was > not > the intended behavior. It was supposed to check whether user made some > changes > and only apply the new version if the template was not touched. Sadly, the > method only checks the name attribute for changes [3], so if "only" > template > content was changed, we still override it. > > While I can try to fix it to originally intended behavior, I'd like to ask > whether it wouldn't be better to use this opportunity and start locking > templates we ship by default. The recommended workflow for users would be > to > clone the template if custom changes are needed. We'd always update locked > templates. Obviously, user would need to merge new version to cloned > template > on his own. With foreman_templates plugin it should be easy enough to > export > templates and see the diff between default and customized template, apply > the > changes user wants and then reimport them back. > > I think this would be overall better user experience and safer workflow. > The > originally intended behavior would never update a template that user > modified. > That means after update user ends up with template from old Foreman > version > (with custom changes) that might not be compatible with the new Foreman > version. This is more likely to happen than before because we now version > templates in community-repo and we don't keep backward compatibility as we > did > before. > > Thanks for reading, thoughts? > > [1] > https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provisioning_templates.rb#L98 > [2] https://github.com/theforeman/foreman/commit/ > d4ed70154fa9f6c83597adc784240e3865845563 > > [3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33 > > -- > Marek >

+1 to locking. This is the only workflow that makes sense imho.
Please note, that we need [1] merged first.

  • Timo

[1] https://github.com/theforeman/foreman/pull/4283

··· Am Freitag, 17. Februar 2017 15:05:13 UTC+1 schrieb Marek Hulán: > > Hello foreman-devs, > > recently I was told about the bug that we override all templates in > database > whenever we run db:seed. From the code [1] and commit message [2], it was > not > the intended behavior. It was supposed to check whether user made some > changes > and only apply the new version if the template was not touched. Sadly, the > method only checks the name attribute for changes [3], so if "only" > template > content was changed, we still override it. > > While I can try to fix it to originally intended behavior, I'd like to ask > whether it wouldn't be better to use this opportunity and start locking > templates we ship by default. The recommended workflow for users would be > to > clone the template if custom changes are needed. We'd always update locked > templates. Obviously, user would need to merge new version to cloned > template > on his own. With foreman_templates plugin it should be easy enough to > export > templates and see the diff between default and customized template, apply > the > changes user wants and then reimport them back. > > I think this would be overall better user experience and safer workflow. > The > originally intended behavior would never update a template that user > modified. > That means after update user ends up with template from old Foreman > version > (with custom changes) that might not be compatible with the new Foreman > version. This is more likely to happen than before because we now version > templates in community-repo and we don't keep backward compatibility as we > did > before. > > Thanks for reading, thoughts? > > [1] > https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provisioning_templates.rb#L98 > [2] https://github.com/theforeman/foreman/commit/ > d4ed70154fa9f6c83597adc784240e3865845563 > > [3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33 > > -- > Marek >

Oh :frowning:

It looks like the fix would be just to pass the template attribute
to audit_modified?, it can check other attributes than name.

But locking the templates we ship would be preferable to me.

  • Stephen
··· On Fri, Feb 17, 2017 at 9:04 AM, Marek Hulán wrote: > Hello foreman-devs, > > recently I was told about the bug that we override all templates in database > whenever we run db:seed. From the code [1] and commit message [2], it was not > the intended behavior. It was supposed to check whether user made some changes > and only apply the new version if the template was not touched. Sadly, the > method only checks the name attribute for changes [3], so if "only" template > content was changed, we still override it. > > While I can try to fix it to originally intended behavior, I'd like to ask > whether it wouldn't be better to use this opportunity and start locking > templates we ship by default. The recommended workflow for users would be to > clone the template if custom changes are needed. We'd always update locked > templates. Obviously, user would need to merge new version to cloned template > on his own. With foreman_templates plugin it should be easy enough to export > templates and see the diff between default and customized template, apply the > changes user wants and then reimport them back. > > I think this would be overall better user experience and safer workflow. The > originally intended behavior would never update a template that user modified. > That means after update user ends up with template from old Foreman version > (with custom changes) that might not be compatible with the new Foreman > version. This is more likely to happen than before because we now version > templates in community-repo and we don't keep backward compatibility as we did > before. > > Thanks for reading, thoughts? > > [1] https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provisioning_templates.rb#L98 > [2] https://github.com/theforeman/foreman/commit/ > d4ed70154fa9f6c83597adc784240e3865845563 > [3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33 > > -- > Marek > > -- > You received this message because you are subscribed to the Google Groups "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

Timo Goebel <mail@timogoebel.name> writes:

> +1 to locking. This is the only workflow that makes sense imho.
> Please note, that we need [1] merged first.

+1 - let's set the right expectations

– Ivan

··· > > - Timo > > [1] https://github.com/theforeman/foreman/pull/4283 > > Am Freitag, 17. Februar 2017 15:05:13 UTC+1 schrieb Marek Hulán: >> >> Hello foreman-devs, >> >> recently I was told about the bug that we override all templates in >> database >> whenever we run db:seed. From the code [1] and commit message [2], it was >> not >> the intended behavior. It was supposed to check whether user made some >> changes >> and only apply the new version if the template was not touched. Sadly, the >> method only checks the name attribute for changes [3], so if "only" >> template >> content was changed, we still override it. >> >> While I can try to fix it to originally intended behavior, I'd like to ask >> whether it wouldn't be better to use this opportunity and start locking >> templates we ship by default. The recommended workflow for users would be >> to >> clone the template if custom changes are needed. We'd always update locked >> templates. Obviously, user would need to merge new version to cloned >> template >> on his own. With foreman_templates plugin it should be easy enough to >> export >> templates and see the diff between default and customized template, apply >> the >> changes user wants and then reimport them back. >> >> I think this would be overall better user experience and safer workflow. >> The >> originally intended behavior would never update a template that user >> modified. >> That means after update user ends up with template from old Foreman >> version >> (with custom changes) that might not be compatible with the new Foreman >> version. This is more likely to happen than before because we now version >> templates in community-repo and we don't keep backward compatibility as we >> did >> before. >> >> Thanks for reading, thoughts? >> >> [1] >> https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provisioning_templates.rb#L98 >> [2] https://github.com/theforeman/foreman/commit/ >> d4ed70154fa9f6c83597adc784240e3865845563 >> >> [3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33 >> >> -- >> Marek >> > > -- > You received this message because you are subscribed to the Google Groups "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

I don't think that would fix it. The attribute can be used to limit the base
of audits that we are searching for - e.g. only templates which content has
changed, but the method still checks whether name has changed at the end.

So it seems we have +1 from everyone so far, I'll try to find some time and
send a PR this week.

Thanks everyone involved

··· -- Marek

On úterý 21. února 2017 8:56:15 CET Stephen Benjamin wrote:

Oh :frowning:

It looks like the fix would be just to pass the template attribute
to audit_modified?, it can check other attributes than name.

But locking the templates we ship would be preferable to me.

  • Stephen

On Fri, Feb 17, 2017 at 9:04 AM, Marek Hulán mhulan@redhat.com wrote:

Hello foreman-devs,

recently I was told about the bug that we override all templates in
database whenever we run db:seed. From the code [1] and commit message
[2], it was not the intended behavior. It was supposed to check whether
user made some changes and only apply the new version if the template was
not touched. Sadly, the method only checks the name attribute for changes
[3], so if “only” template content was changed, we still override it.

While I can try to fix it to originally intended behavior, I’d like to ask
whether it wouldn’t be better to use this opportunity and start locking
templates we ship by default. The recommended workflow for users would be
to clone the template if custom changes are needed. We’d always update
locked templates. Obviously, user would need to merge new version to
cloned template on his own. With foreman_templates plugin it should be
easy enough to export templates and see the diff between default and
customized template, apply the changes user wants and then reimport them
back.

I think this would be overall better user experience and safer workflow.
The originally intended behavior would never update a template that user
modified. That means after update user ends up with template from old
Foreman version (with custom changes) that might not be compatible with
the new Foreman version. This is more likely to happen than before
because we now version templates in community-repo and we don’t keep
backward compatibility as we did before.

Thanks for reading, thoughts?

[1]
https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provision
ing_templates.rb#L98 [2] https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563
[3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33


Marek


You received this message because you are subscribed to the Google Groups
"foreman-dev" group. To unsubscribe from this group and stop receiving
emails from it, send an email to
foreman-dev+unsubscribe@googlegroups.com. For more options, visit
https://groups.google.com/d/optout.

> Timo Goebel <mail@timogoebel.name> writes:
>
> > +1 to locking. This is the only workflow that makes sense imho.
> > Please note, that we need [1] merged first.
>
> +1 - let's set the right expectations
>

I think its +1 across, the main question i have is is a minor Y version
bump is enough or does this fall into the category of 2.0 release?

Ohad

··· On Mon, Feb 20, 2017 at 4:58 PM, Ivan Necas wrote:

– Ivan

  • Timo

[1] https://github.com/theforeman/foreman/pull/4283

Am Freitag, 17. Februar 2017 15:05:13 UTC+1 schrieb Marek Hulán:

Hello foreman-devs,

recently I was told about the bug that we override all templates in
database
whenever we run db:seed. From the code [1] and commit message [2], it
was

not
the intended behavior. It was supposed to check whether user made some
changes
and only apply the new version if the template was not touched. Sadly,
the

method only checks the name attribute for changes [3], so if "only"
template
content was changed, we still override it.

While I can try to fix it to originally intended behavior, I’d like to
ask

whether it wouldn’t be better to use this opportunity and start locking
templates we ship by default. The recommended workflow for users would
be

to
clone the template if custom changes are needed. We’d always update
locked

templates. Obviously, user would need to merge new version to cloned
template
on his own. With foreman_templates plugin it should be easy enough to
export
templates and see the diff between default and customized template,
apply

the
changes user wants and then reimport them back.

I think this would be overall better user experience and safer workflow.
The
originally intended behavior would never update a template that user
modified.
That means after update user ends up with template from old Foreman
version
(with custom changes) that might not be compatible with the new Foreman
version. This is more likely to happen than before because we now
version

templates in community-repo and we don’t keep backward compatibility as
we

did
before.

Thanks for reading, thoughts?

[1]
https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.
d/07-provisioning_templates.rb#L98

[2] https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563
<https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563>

[3] https://github.com/theforeman/foreman/blob/1.14-stable/db/
seeds.rb#L33


Marek


You received this message because you are subscribed to the Google
Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Since we override all templates atm, I think it's not a big change. We're
(sadly) not dropping backwards compatibility. Templates can be unlocked by
admin or users with proper permission, but it would be opt-in now. And we
should document this workflow in release notes and manual ofc.

··· -- Marek

On pondělí 20. února 2017 17:35:59 CET Ohad Levy wrote:

On Mon, Feb 20, 2017 at 4:58 PM, Ivan Necas inecas@redhat.com wrote:

Timo Goebel mail@timogoebel.name writes:

+1 to locking. This is the only workflow that makes sense imho.
Please note, that we need [1] merged first.

+1 - let’s set the right expectations

I think its +1 across, the main question i have is is a minor Y version
bump is enough or does this fall into the category of 2.0 release?

Ohad

– Ivan

  • Timo

[1] https://github.com/theforeman/foreman/pull/4283

Am Freitag, 17. Februar 2017 15:05:13 UTC+1 schrieb Marek Hulán:

Hello foreman-devs,

recently I was told about the bug that we override all templates in
database
whenever we run db:seed. From the code [1] and commit message [2], it

was

not
the intended behavior. It was supposed to check whether user made some
changes
and only apply the new version if the template was not touched. Sadly,

the

method only checks the name attribute for changes [3], so if "only"
template
content was changed, we still override it.

While I can try to fix it to originally intended behavior, I’d like to

ask

whether it wouldn’t be better to use this opportunity and start locking
templates we ship by default. The recommended workflow for users would

be

to
clone the template if custom changes are needed. We’d always update

locked

templates. Obviously, user would need to merge new version to cloned
template
on his own. With foreman_templates plugin it should be easy enough to
export
templates and see the diff between default and customized template,

apply

the
changes user wants and then reimport them back.

I think this would be overall better user experience and safer
workflow.
The
originally intended behavior would never update a template that user
modified.
That means after update user ends up with template from old Foreman
version
(with custom changes) that might not be compatible with the new Foreman
version. This is more likely to happen than before because we now

version

templates in community-repo and we don’t keep backward compatibility as

we

did
before.

Thanks for reading, thoughts?

[1]
https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.

d/07-provisioning_templates.rb#L98

[2] https://github.com/theforeman/foreman/commit/
d4ed70154fa9f6c83597adc784240e3865845563
<https://github.com/theforeman/foreman/commit/

d4ed70154fa9f6c83597adc784240e3865845563>

[3] https://github.com/theforeman/foreman/blob/1.14-stable/db/

seeds.rb#L33


Marek


You received this message because you are subscribed to the Google

Groups “foreman-dev” group.

To unsubscribe from this group and stop receiving emails from it, send

an email to foreman-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.


You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

+1 definitely much better experience, we can carry on then and provide
a diff between cloned and original, so user can merge the changes more
easily.