Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validating data #103

Closed
onedr0p opened this issue Jan 20, 2024 · 8 comments
Closed

Validating data #103

onedr0p opened this issue Jan 20, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@onedr0p
Copy link

onedr0p commented Jan 20, 2024

Is your feature request related to a problem? Please describe.

I was wondering if it currently possible to validate data items before the templating happens. I was poking around the docs and I see Tests but not sure if that is what I should be looking at.

Describe the solution you'd like

I would love to be able run validation on user supplied variables from the data or data-vars flags.

For example, I was wondering if something like this pseudo-code was possible:

def addressing(value: str) -> bool:
    try:
        netaddr.IPNetwork(value)
        return True
    except netaddr.core.AddrFormatError:
        return False

class Loader:
    def filters(self):
        return [nthhost, encrypt]

    def tests(self):
        return [
            addressing(self.data.bootstrap_cluster_cidr),
            addressing(self.data.bootstrap_service_cidr)
        ]
@onedr0p onedr0p added the enhancement New feature or request label Jan 20, 2024
@mirkolenz
Copy link
Owner

Nice idea! Jinja tests have a different purpose, they enable things like {% data.id is my_test_name %}. Since validation is quite different between use cases, I think it may not make much sense to add a dedicated layer to the custom loaders. Instead, I recommend something like the following:

def validate_addressing(value: str) -> None:
    try:
        netaddr.IPNetwork(value)
    except netaddr.core.AddrFormatError as e:
        raise ValueError("Help message") from e


class Loader:
    def __init__(self, data):
        validate_addressing(data.bootstrap_cluster_cidr)
        validate_addressing(data.bootstrap_service_cidr)

In essence, you pass the data you receive in the init function to some custom function that then checks your constraints and raises an exception if necessary. You may also alter the logic (e.g., print some message and then exit the program instead of raising a new exception), so feel free to adjust this snippet as needed. Does that fulfill your use case?

@onedr0p
Copy link
Author

onedr0p commented Jan 20, 2024

I believe this will fit my usecase! I was noodling last night with the __init__ method but could get objects under data to be defined. Using the example you posted I get this error:

Traceback (most recent call last):
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/bin/makejinja", line 8, in <module>
    sys.exit(makejinja_cli())
             ^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/rich_click/rich_command.py", line 126, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/typed_settings/cli_click.py", line 247, in new_func
    return f(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/makejinja/cli.py", line 37, in makejinja_cli
    makejinja(config)
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/makejinja/app.py", line 43, in makejinja
    process_loader(loader, env, data)
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/.venv/lib/python3.11/site-packages/makejinja/app.py", line 314, in process_loader
    loader = cls(**params)
             ^^^^^^^^^^^^^
  File "/Users/devin/Code/repos/onedr0p/flux-template-cluster/bootstrap/scripts/loader.py", line 27, in __init__
    validate_addressing(data.bootstrap_cluster_cidr)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'dict' object has no attribute 'bootstrap_cluster_cidr'
task: Failed to run task "configure": exit status 1

@mirkolenz
Copy link
Owner

mirkolenz commented Jan 20, 2024

Yeah, that exception makes sense. data is a dict, so one needs to access it like so:

data["bootstrap_cluster_cidr"]["maybe_another_nested_attribute"]

Jinja supports the dot-based notation, but pure Python code sadly does not. Sorry for the mistake.

@onedr0p
Copy link
Author

onedr0p commented Jan 20, 2024

Ah I was scratching my head trying to figure out why that wasn't working but makes total sense. Thanks

This is great and definitely covers my usecase <3

@onedr0p onedr0p closed this as completed Jan 20, 2024
@onedr0p
Copy link
Author

onedr0p commented Jan 21, 2024

I wanted to share my progress with this in you are interested. It is coming along very nice: onedr0p/cluster-template#1214

If you would be okay with this I would love to PR my repo to your readme under a "Who's using makejinja?" section at the bottom. I know I would be very happy to look at how other people are using makejinja if I discovered you repo for the first time 😄

@mirkolenz
Copy link
Owner

Thanks for sharing, I am quite surprised that you were able to completely switch from Ansible to makejinja!

I would appreciate such a PR. At the moment, I am making some smaller modifications to the README locally, I will let you know once I am finished so you can create the PR 😄

@mirkolenz
Copy link
Owner

Another area where you could help me: I would love to add a small comparison between Ansible and makejinja to the readme so that others know what separates Ansible templates from this project. Since you used these quite extensively and now have some experience with makejinja I would love to hear about the most important reasons for your switch. Feel free to contact me privately (my mail is stated in my GitHub profile) or open a discussion in this repo.

@mirkolenz
Copy link
Owner

@onedr0p I just started a new discussion (#106) to collect projects using makejinja. In addition, we can still add some featured projects to the readme, but I hope that giving an easy way for people to link to their repo is an advantage and lowers the barrier.

I also just the changes to the README, I think the "featured projects" should ideally be added between "highlights" and "installation".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants