explorer reuse between types #148

New Issue

ungleich-gitea · 2021-11-20T15:20:14Z

ungleich-gitea commented

2021-11-20 15:20:14 +00:00

Created by: antifob

I made PR #557 to not rely solely on getent(1) to get passwd entries (among others) and
just noticed there is overlap with other types' explorers (e.g. __ssh_dot_ssh). As such, I am
wondering whether it would not be more appropriate to allow types to share explorers rather
than having to duplicate code on all changes..

Placing them in __global/explorer would make them run automatically, which is obviously not
desired.

I am unclear on the solution here. Would a small refactoring with symlinks be sufficient or
would a type/share/explorer -style solution be more appropriate? Assuming explorers' interface
are well-defined, I am of the opinion that having a repository of explorers usable by types would
be much more interesting.

*Created by: antifob* I made PR #557 to not rely solely on getent(1) to get passwd entries (among others) and just noticed there is overlap with other types' explorers (e.g. __ssh_dot_ssh). As such, I am wondering whether it would not be more appropriate to allow types to share explorers rather than having to duplicate code on all changes.. Placing them in __global/explorer would make them run automatically, which is obviously not desired. I am unclear on the solution here. Would a small refactoring with symlinks be sufficient or would a type/share/explorer -style solution be more appropriate? Assuming explorers' interface are well-defined, I am of the opinion that having a repository of explorers usable by types would be much more interesting.

ungleich-gitea added the

Stale

label 2021-11-20 15:20:14 +00:00

ungleich-gitea closed this issue

2021-11-20 15:20:14 +00:00

ungleich-gitea commented

2021-11-20 15:43:30 +00:00

closed

ungleich-gitea commented

2021-11-20 15:43:31 +00:00

Created by: antifob

@darko-poljak No offense, but no thanks, I am getting a heavy (many, many seconds) response time trying to hit the page, plus I won't receive a confirmation.

Anyways, #512 is about clean code while the issue I raised is about maintenance. Although I understand and agree with the fundamentals of #512 , transparent execution of "lib/default" vs "lib/$os" could lead, from where I stand, to debugging problems. Already, some explorers and gencode-remote scripts make way too much assumptions about the target host without proper checks. As an example:

        if [ "$init" = 'systemd' ]; then
            # this handles ALL linux distros with systemd
            # e.g. archlinux, gentoo in some cases, new RHEL and SLES versions
            echo "systemctl -q enable \"$name\""
        else
            case "$os" in
                debian)
                    case "$os_version" in
                        [1-7]*)
                            echo "update-rc.d \"$name\" defaults >/dev/null"
                        ;;
                        8*)
                            echo "systemctl enable \"$name\""
                        ;;
                        *)
                            echo "Unsupported version $os_version of $os" >&2
                            exit 1
                        ;;
                    esac
                ;;

which tries to call systemctl if systemd is not the init (Debian 8 [no 9?, also sid has update-rc.d still]).

So having to check per-os, per-cases just leads to duplicated code once, in my opinion... unless most of the code falls into "default", where you would have the same "spaghetti code" again.

I much prefer having a single operation implemented in a single place with proper formatting than having the check where and why a particular block of code was executed.

In any case, I am open to the discussion.

*Created by: antifob* @darko-poljak No offense, but no thanks, I am getting a heavy (many, many seconds) response time trying to hit the page, plus I won't receive a confirmation. Anyways, #512 is about clean code while the issue I raised is about maintenance. Although I understand and agree with the fundamentals of #512 , transparent execution of "lib/default" vs "lib/$os" could lead, from where I stand, to debugging problems. Already, some explorers and gencode-remote scripts make way too much assumptions about the target host without proper checks. As an example: ``` if [ "$init" = 'systemd' ]; then # this handles ALL linux distros with systemd # e.g. archlinux, gentoo in some cases, new RHEL and SLES versions echo "systemctl -q enable \"$name\"" else case "$os" in debian) case "$os_version" in [1-7]*) echo "update-rc.d \"$name\" defaults >/dev/null" ;; 8*) echo "systemctl enable \"$name\"" ;; *) echo "Unsupported version $os_version of $os" >&2 exit 1 ;; esac ;; ``` which tries to call systemctl if systemd is not the init (Debian 8 [no 9?, also sid has update-rc.d still]). So having to check per-os, per-cases just leads to duplicated code once, in my opinion... unless most of the code falls into "default", where you would have the same "spaghetti code" again. I much prefer having a single operation implemented in a single place with proper formatting than having the check where and why a particular block of code was executed. In any case, I am open to the discussion.

ungleich-gitea commented

2021-11-20 15:43:33 +00:00

Created by: darko-poljak

@asteven @uqam-fob @telmich Perhaps this can be connected with this idea: https://github.com/ungleich/cdist/issues/512

*Created by: darko-poljak* @asteven @uqam-fob @telmich Perhaps this can be connected with this idea: https://github.com/ungleich/cdist/issues/512

ungleich-gitea commented

2021-11-20 15:43:34 +00:00

Created by: darko-poljak

@uqam-fob Please join ungleich's cdist chat channel: https://brandnewchat.ungleich.ch/ungleich/channels/cdist, a place for main cdist discussion.

*Created by: darko-poljak* @uqam-fob Please join ungleich's cdist chat channel: https://brandnewchat.ungleich.ch/ungleich/channels/cdist, a place for main cdist discussion.

ungleich-gitea commented

2021-11-20 15:43:35 +00:00

Created by: antifob

Wonderful!

I implemented a small proof of concept in: d5c1a7cd77

*Created by: antifob* Wonderful! I implemented a small proof of concept in: https://github.com/uqam-fob/cdist/commit/d5c1a7cd7740ce30fff41b6c077257687eaa6546

ungleich-gitea commented

2021-11-20 15:43:37 +00:00

Created by: telmich

That is an interesting question! We so far have not had that much and thus not the need for a solution, however we implemented __files lately (maybe a year ago) and maybe we could either use that or some other location to provide for "reusable explorers".

We might though want to even go one step further and @asteven will be very awake when he reads this and maybe create something like a "shell library".

The discussion is open, I would say! :-)

*Created by: telmich* That is an interesting question! We so far have not had that much and thus not the need for a solution, however we implemented __files lately (maybe a year ago) and maybe we could either use that or some other location to provide for "reusable explorers". We might though want to even go one step further and @asteven will be very awake when he reads this and maybe create something like a "shell library". The discussion is open, I would say! :-)

Sign in to join this conversation.