-
Notifications
You must be signed in to change notification settings - Fork 19
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
Make new inventory.py pass pylint #591
Comments
@owendelong I see this file /ansible/LibInventory.pm in the repo. It looks unused and commit suggests it was for KEA. Since we're configuring Kea with Nix, I don't believe we need this file. Can you please confirm? Only reason I ask is that @sarcasticadmin and I are going to be 1. removing this /ansible directory soon and 2. modifying the data structure that inventory.py produces. I want to make sure I don't break anything you have integrated or planned. Thank you! |
I wasn’t involved in our Kea implementation before or with NIX, so actually, Rob is the one to ask this, primarily.
I think this was intended to make the inventory.py output accessible to PERL scripts. As long as the output is
In JSON, this perl script is data agnostic (it’s super simple and basically a wrapper around decode_json() from
The JSON.PM package.
I don’t see anything using it:
delong-dhcp162:owen (128) ~/SCALE/networking/scale_repo/scale-network % grep -ri LibInventory .
Binary file ./.git/objects/pack/pack-321bec2ab4c7e511bcd2176526df353c12c58a1e.pack matches
Binary file ./.git/index matches
(That’s against current Master with some slight tweaks I made to LibInventory.pm to check it):
delong-dhcp162:owen (129) ~/SCALE/networking/scale_repo/scale-network % git status
On branch master
Your branch is up to date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: ansible/LibInventory.pm
no changes added to commit (use "git add" and/or "git commit -a")
delong-dhcp162:owen (130) ~/SCALE/networking/scale_repo/scale-network % git diff
diff --git a/ansible/LibInventory.pm b/ansible/LibInventory.pm
index 00d353f..68d2c61 100755
--- a/ansible/LibInventory.pm
+++ b/ansible/LibInventory.pm
@@ -4,8 +4,9 @@
#
use strict;
-#use JSON;
+use JSON;
use Data::Dumper;
+main();
sub main
{
@@ -24,6 +25,6 @@ sub main
sub JSON_Parse
{
my $string = shift @_;
-# return(decode_json($string));
+ return(decode_json($string));
}
So I think you’re free to remove the directory. I would like to keep the script around because it provides a convenient way to produce a human readable version of what’s inventory.py produces. I’ll go ahead and add this to a PR.
PR submitted and ready for review.
Owen
… On Sep 21, 2023, at 11:18, kylerisse ***@***.***> wrote:
@owendelong <https://github.com/owendelong> I see this file /ansible/LibInventory.pm <https://github.com/socallinuxexpo/scale-network/blob/master/ansible/LibInventory.pm> in the repo. It looks unused and commit suggests it was for KEA. Since we're configuring Kea with Nix, I don't believe we need this file. Can you please confirm? Only reason I ask is that @sarcasticadmin <https://github.com/sarcasticadmin> and I are going to be 1. removing this /ansible directory soon and 2. modifying the data structure that inventory.py <https://github.com/socallinuxexpo/scale-network/blob/master/facts/inventory.py> produces. I want to make sure I don't break anything you have integrated or planned. Thank you!
—
Reply to this email directly, view it on GitHub <#591 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAK6GTRA567RWDJHEK6H52TX3SAILANCNFSM6AAAAAAVZQ7UCM>.
You are receiving this because you were mentioned.
|
Cool, yea all the Nix KEA stuff is strait forward enough. This commit made it look like you had intended to do Kea stuff with perl prior to the Nix effort, only reason I asked. I had already grepped around. We’ll add human readable output as part of the new work. We’ll get it all squared away next week. Thanks Owen! |
not sure if this relevant, but nix can output to JSON. might be interest to capture this data, and do something with it, like comparing a machine against a baseline or a single source of truth. |
This topic has wandered a bit, but IIRC, inventory.py is now deprecated and the core issues discussed have been resolved. If anyone believes this is wrong, please re-open this issue. |
Description
In order to quickly iterate on the NixOS migration, @sarcasticadmin added
scale-network/facts/inventory.py
Line 2 in ffda511
Acceptance Criteria
# pylint: skip-file
removed from/facts/inventory.py
/facts/inventory.py
passes pylintThe text was updated successfully, but these errors were encountered: