Todd/gooey by toddatctera · Pull Request #32 · ctera/ctools

toddatctera · 2021-10-06T06:22:25Z

Change from input-driven menu system to argument based program so we can build an easy GUI using Gooey library.
Added new tasks and functionality.

WIP for #23. Only works with get status function and only after hardcoding the output filename.

Added a subparser to lay groundwork for each task having its own page with required arguments like filename for get_status.

Technically this works but I have to select the task twice because of how I previously was calling functions with the FUNCTION_MAP. So still a WIP but still an improvement for #23

Tasks now show up in the side bar and don't have to be selected twice. Logs look good. Can also be run from the CLI with the flag `--ignore-gooey` and it works! Example: python ctools.py --ignore-gooey run_cmd 'dbg level debug' portal.example.com admin p@ssw0rd TODO: - Add docstrings for Gooey code - Add the remaining tasks like enable ssh

Added portal_parent_parser since each task so far requires logging into the portal. This allows us to re-use the parent parser in a all sub-parsers which are the tasks. Stolen from here: https://stackoverflow.com/questions/33645859/how-to-add-common-arguments-to-argparse-subcommands Also got the -v/--verbose flag working in CLI (--ignore-gooey) mode in GUI mode. In not specified or left unchecked, uses INFO level. TODO: - Add enable_telnet to tasks again - Cleanup and document everything well - Test, test, test - Think about code review and merging.

Added setup steps for Windows using PowerShell. Updated requirements to include Gooey for the GUI. Added easy CLI mode based on below comment chriskiehl/Gooey#449 (comment)

Renamed unlock function to enable_telnet. Changed logic from interactive to parameter based since Gooey cannot finish if a function calls input().

For #26, this adds a check and logs a warning if the admin signing in does not have 'Allow Single Sign on to Devices' checked for the Read Write Admin role in the Administration tenant.

Updated docstring in ctools.py, added optional --all flag which will run the previoiusly default function of getting all connected filers to a portal globally which is slow for large deployments. Commented out the function to browse the admin portal. By doing the above, we will now default to browsing the tenant based on the current session and only browse to Administration if the --all flag is set which sets all_tenants to True in filer.py. In filer.py, I added tenant name column and checking if the output file already exists, if it does, we skip writing the header and just append to the file. I also added try statements to some values that were throwing exceptions on a 6.0.247 filer.

Created a function, get_ad_status() that stores the result of status.fileservices.cifs.joinStatus and parses the result. If it's -1, it's on a workgroup. If 0, return Ok. Else, return Failed.

Requires Filer is connected to portal like all our tasks currently. Also increased the size of the GUI so no scrolling is required and tweaked our program description.

run_cmd.py -- Add granularity to target one or more filers. ctools.py -- Add new run_cmd args. Add logout and exit log msg. filer.py -- Remove input() calls as they're no longer used. By default, it will now run the command on all Filers of the current tenant. Added optional -all flag to allow running against all Filers globally as it worked before or --device flag if provided it will only run on that filer.

Resolve #29. ctools.py -- add optional ignore_cert flag to portal parser login.py -- refactored. Added exception handler, device_sso check, disable ssl warnings if ignore_cert flag is passed run_cmd.py -- phrasing unlock.py -- removed exit that caused an apparent crash in GUI README.md -- updated LICENSE.md -- added

ygalblum

As I wrote you separately, you should run pylint and flake8. For example, your missing a space between the comma and the next parameter in several places

ygalblum · 2021-10-06T18:17:44Z

filer.py

@@ -3,10 +3,6 @@

 def get_filer(self,device=None,tenant=None):
    """Prompt for Filer and Return Filer object if found"""


I guess the comment is no longer relevant, right?

Indeed. There should be no more prompt() functions or comments suggesting there's a prompt.
I'm not even using this function at this point.

ygalblum · 2021-10-06T18:19:04Z

ctools.py


-def set_logging(p_level=logging.INFO,log_file="log.txt"):
+""" If any args are present, run in CLI mode"""
+if len(sys.argv) >= 2:


Move this code into the main method (or the main handler). You don't want any code to run from a mere import

Understood but this check doesn't work when I move it into main as mentioned here:
chriskiehl/Gooey#449 (comment)

I could try this approach:
chriskiehl/Gooey#296 (comment)

Or I can just remove this check and use --ignore-gooey flag instead when I want to run it from CLI.
I found that an added benefit to this is that I can add use_cmd_args=True to the @Gooey decorator and then pass any arguments into the GUI when launched. I think this could be a better feature in the long run.

filer.py

ygalblum · 2021-10-06T18:27:49Z

filer.py

+            all_filers = self.devices.filers(include=[
+                    'deviceConnectionStatus.connected',
+                    'deviceReportedStatus.config.hostname'])
+            for filer in all_filers:


You should use list comprehension instead of looping and extend the array.
connected_filers.extend([filer for filer in all_filers if filer.deviceConnectionStatus.connected])
The same goes for both the "if" and the "else"

Will do. Thanks for example code.

ygalblum · 2021-10-06T18:30:14Z

login.py

 import logging, sys
+import urllib3
+
+from cterasdk import *


Unlike what the documentation says. Generally in Python it's better to import only what you need and not "*".
Assuming I didn't miss anything all you you need in this file are config, GlobalAdmin and CTERAException

ygalblum · 2021-10-06T18:31:21Z

reset_password.py

@@ -0,0 +1,18 @@
+from filer import get_filer
+from cterasdk import *


Same comment. All you need to import here is the CTERAException

ygalblum · 2021-10-06T18:34:31Z

status.py

+        try:
+            selfScanIntervalInHours = info.config.cloudsync.selfScanVerificationIntervalInHours
+        except:
+            selfScanIntervalInHours = ('Not Applicable')


Through this method, the parenthesis are not needed when you have only a string in them

ygalblum · 2021-10-06T18:40:38Z

unlock.py

    if pubkey:
-        print("Copying existing public key to",filer.name)
+        logging.info("Copying provided public key to Filer: {}.".format(device_name))
        filer.ssh.enable(pubkey)


The public key parameter is not positional. So, it's safer to pass it with its name (public_key=pubkey). This way if the API changes and a positional parameter is added, Python won't use this parameter as the first positional one

Got it. Thanks for the explanation.

ygalblum · 2021-10-06T18:42:10Z

unlock.py

+        logging.info("Creating new public/private key pair for Filer {} in $HOME\Downloads."
+                .format(device_name))
        try:
            filer.ssh.enable()


There's no point for this check since the API does the same. If both public_key and public_key_file are None, the SDK will create the keys

Sounds good. Can the API check if a provided public_key is valid?

ygalblum · 2021-10-06T18:45:19Z

Please note that you will need to rebase this branch on top of the master one and fix the conflicts. Otherwise, you won't be able to merge the code. While at it, squash all the commits into one since you don't want to have points in your tree that are broken (and fixed in a following commit)

And other general cleanup or minor refactors of functions.

login.py

Closes issues #29, #26, #23, #14, #5, and #4. Closes PR #32 Squashed commit of the following: commit b4df6d7 Author: Todd Butters <todd@ctera.com> Date: Sun Oct 17 00:22:59 2021 -0400 Pass 99% of pylint and flake8 checks. Add configs. commit ab16230 Author: Todd Butters <todd@ctera.com> Date: Wed Oct 13 10:29:23 2021 -0400 Import cterasdk config for setting SSL trust. commit 8a399ed Author: Todd Butters <todd@ctera.com> Date: Fri Oct 8 10:48:53 2021 -0400 Fixed pylint issues and made requested changes. And other general cleanup or minor refactors of functions. commit 3094163 Author: Todd Butters <todd@ctera.com> Date: Wed Oct 6 01:04:54 2021 -0400 Handle invalid certs. Refactor login module. Resolve #29. ctools.py -- add optional ignore_cert flag to portal parser login.py -- refactored. Added exception handler, device_sso check, disable ssl warnings if ignore_cert flag is passed run_cmd.py -- phrasing unlock.py -- removed exit that caused an apparent crash in GUI README.md -- updated LICENSE.md -- added commit 953e08e Author: Todd Butters <todd@ctera.com> Date: Fri Oct 1 17:30:12 2021 -0400 Fixed another GUI error in enable_telnet task. commit 4c780c7 Author: Todd Butters <todd@ctera.com> Date: Fri Oct 1 17:18:13 2021 -0400 Fix error in GUI run_cmd task. Bump minor version. commit 1986f4d Author: Todd Butters <todd@ctera.com> Date: Fri Oct 1 16:58:00 2021 -0400 Run_cmd per device, tenant, or global. run_cmd.py -- Add granularity to target one or more filers. ctools.py -- Add new run_cmd args. Add logout and exit log msg. filer.py -- Remove input() calls as they're no longer used. By default, it will now run the command on all Filers of the current tenant. Added optional -all flag to allow running against all Filers globally as it worked before or --device flag if provided it will only run on that filer. commit b30db2e Author: Todd Butters <todd@ctera.com> Date: Tue Sep 28 10:44:56 2021 -0400 Add task to reset a local Filer user's password. Requires Filer is connected to portal like all our tasks currently. Also increased the size of the GUI so no scrolling is required and tweaked our program description. commit 3831bcc Author: Todd Butters <todd@ctera.com> Date: Fri Sep 24 16:15:11 2021 -0400 Added About and Help menu. commit 27c0705 Author: Todd Butters <todd@ctera.com> Date: Thu Sep 23 23:51:49 2021 -0400 Added screenshot to README from new images folder. commit 4ad2c51 Author: Todd Butters <todd@ctera.com> Date: Thu Sep 23 17:45:27 2021 -0400 Updated setup instructions. commit a0796cd Author: Todd Butters <todd@ctera.com> Date: Thu Sep 23 16:49:23 2021 -0400 Added AD domain join status to Filer status report. Created a function, get_ad_status() that stores the result of status.fileservices.cifs.joinStatus and parses the result. If it's -1, it's on a workgroup. If 0, return Ok. Else, return Failed. commit 6804fb2 Author: Todd Butters <todd@ctera.com> Date: Thu Sep 23 02:04:57 2021 -0400 Get Filer status per tenant or globally. Updated docstring in ctools.py, added optional --all flag which will run the previoiusly default function of getting all connected filers to a portal globally which is slow for large deployments. Commented out the function to browse the admin portal. By doing the above, we will now default to browsing the tenant based on the current session and only browse to Administration if the --all flag is set which sets all_tenants to True in filer.py. In filer.py, I added tenant name column and checking if the output file already exists, if it does, we skip writing the header and just append to the file. I also added try statements to some values that were throwing exceptions on a 6.0.247 filer. commit efd4c6e Author: Todd Butters <todd@ctera.com> Date: Wed Sep 22 17:20:15 2021 -0400 Check if admin has allowSSO to manage devices. For #26, this adds a check and logs a warning if the admin signing in does not have 'Allow Single Sign on to Devices' checked for the Read Write Admin role in the Administration tenant. commit 8871f1b Author: Todd Butters <todd@ctera.com> Date: Mon Sep 20 07:09:01 2021 -0400 Added enable_telnet support in Gooey GUI. Renamed unlock function to enable_telnet. Changed logic from interactive to parameter based since Gooey cannot finish if a function calls input(). commit 794d4dc Author: toddatctera <todd@ctera.com> Date: Mon May 24 23:54:38 2021 -0400 Updated requirements, Windows setup, and easy CLI. Added setup steps for Windows using PowerShell. Updated requirements to include Gooey for the GUI. Added easy CLI mode based on below comment chriskiehl/Gooey#449 (comment) commit 8bb39f3 Author: toddatctera <todd@ctera.com> Date: Wed May 12 17:58:24 2021 -0400 Added parent parsers and verbose toggle. Added portal_parent_parser since each task so far requires logging into the portal. This allows us to re-use the parent parser in a all sub-parsers which are the tasks. Stolen from here: https://stackoverflow.com/questions/33645859/how-to-add-common-arguments-to-argparse-subcommands Also got the -v/--verbose flag working in CLI (--ignore-gooey) mode in GUI mode. In not specified or left unchecked, uses INFO level. TODO: - Add enable_telnet to tasks again - Cleanup and document everything well - Test, test, test - Think about code review and merging. commit bfccba4 Author: toddatctera <todd@ctera.com> Date: Fri May 7 16:12:49 2021 -0400 Functional GUI using Gooey and argparse. Tasks now show up in the side bar and don't have to be selected twice. Logs look good. Can also be run from the CLI with the flag `--ignore-gooey` and it works! Example: python ctools.py --ignore-gooey run_cmd 'dbg level debug' portal.example.com admin p@ssw0rd TODO: - Add docstrings for Gooey code - Add the remaining tasks like enable ssh commit c24314e Author: toddatctera <todd@ctera.com> Date: Fri May 7 03:29:08 2021 -0400 Added run_cmd to Gooey Technically this works but I have to select the task twice because of how I previously was calling functions with the FUNCTION_MAP. So still a WIP but still an improvement for #23 commit bf0c568 Author: toddatctera <todd@ctera.com> Date: Fri May 7 01:42:28 2021 -0400 Added subparser and get_status filename with Gooey. Added a subparser to lay groundwork for each task having its own page with required arguments like filename for get_status. commit ed5d258 Author: Todd Butters <todd@ctera.com> Date: Thu Apr 22 17:50:28 2021 -0400 Initial Gooey POC WIP for #23. Only works with get status function and only after hardcoding the output filename. commit 7696bd7 Author: Todd Butters <47750691+toddatctera@users.noreply.github.com> Date: Thu Apr 22 00:19:09 2021 -0400 Added disable_ssh and Portal creds as arguments.

toddatctera · 2021-10-22T20:36:02Z

@ygalblum , thanks for your help.
I didn't rebase but I think I accomplished about the same thing this way.
I merged and squashed my ~20 or so commits from this branch into main this way:

git switch main
git merge todd/gooey --squash
git rm menu.py
git add ctools.py filer.py login.py run_cmd.py status.py suspend_sync.py unlock.py unsuspend_sync.py
git commit
git push

I think I can close my first PR. Yes?

ygalblum · 2021-10-24T05:38:34Z

@toddatctera I guess that worked for you. But, this is not the right way of doing this. You want to merge your PRs as PRs mean something.
I suggest that next time, you first rebase your code on top of main (by running git rebase main. You'll get conflict errors during the rebase which you will fix one by one and then have a branch you can merge to master. Keep in mind that at that point, your local branch will diverge from the one in the server. So, you'll need to force push it git push -force. Once you do that, the PR will be mergeable.
Lastly, If you want to squash your commits together (I usually do), run the rebase command with -i. This will open a windows showing all the commits for the rebase and you can then choose if you want to squash them together (or into groups, or whatever)

toddatctera and others added 19 commits April 22, 2021 00:19

Added disable_ssh and Portal creds as arguments.

7696bd7

Initial Gooey POC

ed5d258

WIP for #23. Only works with get status function and only after hardcoding the output filename.

Added subparser and get_status filename with Gooey.

bf0c568

Added a subparser to lay groundwork for each task having its own page with required arguments like filename for get_status.

Added run_cmd to Gooey

c24314e

Technically this works but I have to select the task twice because of how I previously was calling functions with the FUNCTION_MAP. So still a WIP but still an improvement for #23

Updated requirements, Windows setup, and easy CLI.

794d4dc

Added setup steps for Windows using PowerShell. Updated requirements to include Gooey for the GUI. Added easy CLI mode based on below comment chriskiehl/Gooey#449 (comment)

Added enable_telnet support in Gooey GUI.

8871f1b

Renamed unlock function to enable_telnet. Changed logic from interactive to parameter based since Gooey cannot finish if a function calls input().

Check if admin has allowSSO to manage devices.

efd4c6e

For #26, this adds a check and logs a warning if the admin signing in does not have 'Allow Single Sign on to Devices' checked for the Read Write Admin role in the Administration tenant.

Added AD domain join status to Filer status report.

a0796cd

Created a function, get_ad_status() that stores the result of status.fileservices.cifs.joinStatus and parses the result. If it's -1, it's on a workgroup. If 0, return Ok. Else, return Failed.

Updated setup instructions.

4ad2c51

Added screenshot to README from new images folder.

27c0705

Added About and Help menu.

3831bcc

Add task to reset a local Filer user's password.

b30db2e

Requires Filer is connected to portal like all our tasks currently. Also increased the size of the GUI so no scrolling is required and tweaked our program description.

Fix error in GUI run_cmd task. Bump minor version.

4c780c7

Fixed another GUI error in enable_telnet task.

953e08e

toddatctera requested review from mj-ctera and ygalblum October 6, 2021 06:23

ygalblum suggested changes Oct 6, 2021

View reviewed changes

Fixed pylint issues and made requested changes.

8a399ed

And other general cleanup or minor refactors of functions.

ygalblum reviewed Oct 10, 2021

View reviewed changes

login.py Outdated Show resolved Hide resolved

toddatctera added 2 commits October 13, 2021 10:29

Import cterasdk config for setting SSL trust.

ab16230

Pass 99% of pylint and flake8 checks. Add configs.

b4df6d7

ygalblum approved these changes Oct 17, 2021

View reviewed changes

lakepry25 closed this Dec 6, 2023

		@@ -3,10 +3,6 @@

		def get_filer(self,device=None,tenant=None):
		"""Prompt for Filer and Return Filer object if found"""

		@@ -0,0 +1,18 @@
		from filer import get_filer
		from cterasdk import *

Conversation

toddatctera commented Oct 6, 2021

Uh oh!

ygalblum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ygalblum commented Oct 6, 2021

Uh oh!

Uh oh!

toddatctera commented Oct 22, 2021

Uh oh!

ygalblum commented Oct 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants