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.

Sign up for GitHub

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

Jump to bottom

[FEATURE REQUEST] tighter structure with hcache data #36

Open
bodleytunes opened this issue Jul 6, 2023 · 12 comments
Open

[FEATURE REQUEST] tighter structure with hcache data #36

bodleytunes opened this issue Jul 6, 2023 · 12 comments

Comments

@bodleytunes
Copy link

When saving data to the Hcache from something like TTP parsing, the nesting includes "run_ttp" as a key.

Is there a way to have the dict it creates in the hcache without the run_ttp key to make it a little easier to navigate / more readable?

           tacacs:
                    ----------
                    run_ttp:
                        |_
                          ----------
                          group:
                              |_
                                ----------
                                name:
                                    TACACS
@dmulyalin
Copy link
Owner

Its a good idea but not feasible to implement without significant code and tests rework, which is not justifiable for such a small convenience improvement.

run_ttp is the name of the task that set by dataprocessor after ttp parsing, task name is mandatory attribute, to remove it from task results will have to introduce non trivial logic in several places in the codebase.

@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 •
edited
Loading

My only slight issue with it is the structure is not always the same. If I run it from the command line like salt nrp1 nr.cli xyz then it gives a slightly different structure than when I run the equivalent command from the workflow steps, which can sometimes cause a bit of head scratching when you try to reference the variable where you think it should be nested, say from a template, and it is in a slightly different position, so when you test from the cli its giving different context than when running the state/workflow. Can't recall if its to do with the result.0 part moving around?

@dmulyalin
Copy link
Owner

ic, could you provide samples of the discrepancies and what was done to produce the results, maybe sample workflow or cli command, would be glad to have a look and see if those can be made to look alike.

@dmulyalin dmulyalin reopened this Aug 22, 2023
@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 •
edited
Loading

OK looking at it there isn't much difference...

From Workflow step:

          tacacs_ips_workflow:
                    ----------
                    run_ttp:
                        ----------
                        changed:
                            False
                        connection_retry:
                            0
                        diff:
                        exception:
                            None
                        failed:
                            False
                        result:
                            |_
                              ----------
                              tacacs:
                                  - 10.0.0.1
                                  - 10.0.0.2
                                  - 10.0.0.3
                              tacacs_group:
                                  TACACS-SERVERS
                              vrf:
                                  MANAGEMENT
                        task_retry:
                            0

From running in the cli

                tacacs_ips_cli:
                    ----------
                    run_ttp:
                        |_
                          ----------
                              tacacs:
                                  - 10.0.0.1
                                  - 10.0.0.2
                                  - 10.0.0.3
                          tacacs_group:
                              TACACS-SERVERS
                          vrf:
                              MANAGEMENT

workflow has run_ttp.result.0.tacacs

cli has a ever so slightly simpler structure of run_ttp.0.tacacs

I assume this is probably because its needed for reporting with the workflows.

@bodleytunes
Copy link
Author

Looking at it its probably nothing worth really bothering about as you said seems like it could be a design change for no real world gains. As long as I remember the differences then should be fine, its just it tripped me up a couple of times, but I've learned now the differences.

@dmulyalin
Copy link
Owner

workflow uses add_details=True for all steps by default, if you run your nr.cli run_ttp task with add_details=True you should get same exact result as workflow does.

add_details docs

@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 via email

@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 via email

@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 via email

@dmulyalin
Copy link
Owner

worlflow has to use add_details=True to be able to reason whether task was successful or failed, so its not controllable from workflow steps arguments.

@bodleytunes
Copy link
Author

bodleytunes commented Aug 22, 2023 via email

@dmulyalin
Copy link
Owner

in other words, workflow sets add_details=True for all nr.xyz step unconditionally -

salt-nornir/salt_nornir/states/nornir_proxy_state_module.py

Line 308 in 1fc3b16

step["kwargs"]["add_details"] = True

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bodleytunes @dmulyalin