Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

load_template improvements #143

Open
ktbyers opened this issue Nov 18, 2016 · 5 comments
Open

load_template improvements #143

ktbyers opened this issue Nov 18, 2016 · 5 comments
Milestone

Comments

@ktbyers
Copy link
Contributor

ktbyers commented Nov 18, 2016

Here are some improvements to load_template() I think we should make.

  1. Support 'replace' operation. I would propose that we add a mode='merge' / mode='replace' parameter.

  2. Have it behave properly if you specify 'my_template.j2' currently it will look for 'my_template.j2.j2'.

  3. Be able to load templates from current working directory (as long as we can make this work in a reasonable way without breaking backwords compatibility).

  4. Improve the docs here, http://napalm.readthedocs.io/en/latest/base.html

I think the behavior of some of the parameters needs clarified.

@ktbyers
Copy link
Contributor Author

ktbyers commented Nov 18, 2016

cc @mirceaulinic

I can work on these, I just wanted a placeholder so I remember things that I saw.

@mirceaulinic
Copy link
Member

Support 'replace' operation. I would propose that we add a mode='merge' / mode='replace' parameter.

I'm into this.

Have it behave properly if you specify 'my_template.j2' currently it will look for 'my_template.j2.j2'.

Funny one :) Yes, we should definitely avoid this.

Be able to load templates from current working directory (as long as we can make this work in a reasonable way without breaking backwords compatibility).
Improve the docs here, http://napalm.readthedocs.io/en/latest/base.html

They all sound good to me @ktbyers! Thanks!

@dbarrosop
Copy link
Member

dbarrosop commented Nov 20, 2016

Have it behave properly if you specify 'my_template.j2' currently it will look for 'my_template.j2.j2'.

The idea behind this was that you would specify you want the ntp template, so it wasn't really meant to be a file. That's why the .j2 was added by the method. I am fine with adding a safety to prevent that from happening, I was just explaining the reason behind it : )

Be able to load templates from current working directory (as long as we can make this work in a reasonable way without breaking backwords compatibility).

If you load the template blah.j2 in your current folder, that code is gonna be bound to the vendor for which you wrote the template for. If you want to add another vendor later on, your code will have to be adapted. What's wrong with requiring people to put their templates in ./templates/$vendor? I am trying to see the use case because I think it's effortless and in the long term it might have benefits for the user without any actual drawback.

@mirceaulinic mirceaulinic added this to the APPROVED milestone Nov 21, 2016
@ktbyers
Copy link
Contributor Author

ktbyers commented Nov 22, 2016

The advantage I see is since a lot of platforms copy Cisco for configuration, you could potentially use the same template across multiple platforms (without resorting to symlinks).

The method was confusing when trying to use it as it took several tries to figure out how to get it to work (i.e. the most logical expectation was you could specify 'my_file.j2' and it would look in current working directory). This could be improved, however, by improving the documentation for this method.

I don't think the loading from current working directory is very important, however...

@dbarrosop
Copy link
Member

That the documentation could be improved nobody doubts it :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants