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

Refactor savenrestore gitlab #21

Merged
merged 24 commits into from
Apr 21, 2017

Conversation

jimbr70
Copy link

@jimbr70 jimbr70 commented Apr 19, 2017

changes to gitlab client, some messaging, various fixzes from merge.


This change is Reviewable

add gitlab
additional template support
…re' into refactor-saveNrestore-GITLAB

# Conflicts:
#	sandbox_scripts/environment/setup/setup_resources.py
#	sandbox_scripts/helpers/Networking/NetworkingSaveNRestore.py
@kalsky
Copy link
Member

kalsky commented Apr 19, 2017

Review status: 0 of 10 files reviewed at latest revision, 10 unresolved discussions, some commit checks failed.


sandbox_scripts/environment/setup/setup_resources.py, line 10 at r1 (raw file):

import os, sys

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.
this is also affecting the tests we're running.

also in the os.environ line you just override the value you got from cloudshell in a real execution so it needs to be removed from this file anyway.


sandbox_scripts/environment/setup/setup_resources.py, line 77 at r1 (raw file):

            self.logger.error("Setup failed. Unexpected error:" + str(sys.exc_info()[0]))

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.


sandbox_scripts/environment/teardown/teardown_resources.py, line 26 at r1 (raw file):

            if saveNRestoreTool.get_storage_manager():
                ignore_models = ['Generic TFTP server', 'Config Set Pool', 'Generic FTP server', 'netscout switch 3912',
                             'OnPATH Switch 3903', 'Ixia Traffic generator', "SubNet-28", "SubNet-30"]

are you missing the gitlab model here?


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 131 at r1 (raw file):

                if command.Name == 'restore':
                    for parm in command.Parameters:
                        print "****" + parm.Name + "****"

this print might appear in the output, we should remove it


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 132 at r1 (raw file):

                    for parm in command.Parameters:
                        print "****" + parm.Name + "****"
                        if parm.Name in "path, src_Path, xx123":

no need for the xx123 i guess (same as yy123, zzz123 below)
also, you're just checking if the param name is inside this string, not inside a list of values like this:
a = ["aaa","bbb","ccc"]
if "bbb" in a:


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 140 at r1 (raw file):

            if the_path == "undef" or the_cfgtype == "undef" or the_restoremeth == "undef":
                raise "Failed to find viable restore command for " + self.name \

why not raising a QualiError?


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 144 at r1 (raw file):

        except:
            raise "Failed building restore command input parm names."

why not raising a QualiError?


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 150 at r1 (raw file):

                              InputNameValue(the_cfgtype, str(config_type)),
                              InputNameValue(the_restoremeth, str(restore_method))]
            print "resource.py L124 " + self.name + "  configuration_type = " + str(config_type)

remove any (debug) prints, they might appear in the output


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 162 at r1 (raw file):

        except QualiError as qerror:
            print "ERROR1: Load config file failed. " + qerror.message

remove (debug) prints


sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 167 at r1 (raw file):

        except:
            print "ERROR2: Load config file failed. "

remove (debug) prints


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/environment/setup/setup_resources.py, line 10 at r1 (raw file):

Previously, kalsky wrote…

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.
this is also affecting the tests we're running.

also in the os.environ line you just override the value you got from cloudshell in a real execution so it needs to be removed from this file anyway.

change made


Comments from Reviewable

@kalsky
Copy link
Member

kalsky commented Apr 20, 2017

sandbox_scripts/environment/setup/setup_resources.py, line 10 at r1 (raw file):

Previously, kalsky wrote…

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.
this is also affecting the tests we're running.

also in the os.environ line you just override the value you got from cloudshell in a real execution so it needs to be removed from this file anyway.

OK


Comments from Reviewable

@kalsky
Copy link
Member

kalsky commented Apr 20, 2017

sandbox_scripts/environment/setup/setup_resources.py, line 77 at r1 (raw file):

Previously, kalsky wrote…

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.

OK


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/environment/teardown/teardown_resources.py, line 26 at r1 (raw file):

Previously, kalsky wrote…

are you missing the gitlab model here?

Done. change made


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 131 at r1 (raw file):

Previously, kalsky wrote…

this print might appear in the output, we should remove it

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 132 at r1 (raw file):

Previously, kalsky wrote…

no need for the xx123 i guess (same as yy123, zzz123 below)
also, you're just checking if the param name is inside this string, not inside a list of values like this:
a = ["aaa","bbb","ccc"]
if "bbb" in a:

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 140 at r1 (raw file):

Previously, kalsky wrote…

why not raising a QualiError?

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 144 at r1 (raw file):

Previously, kalsky wrote…

why not raising a QualiError?

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 150 at r1 (raw file):

Previously, kalsky wrote…

remove any (debug) prints, they might appear in the output

Done.


Comments from Reviewable

@kalsky
Copy link
Member

kalsky commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 132 at r1 (raw file):

Previously, jimbr70 wrote…

Done.

can't see it that it's done


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 162 at r1 (raw file):

Previously, kalsky wrote…

remove (debug) prints

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 167 at r1 (raw file):

Previously, kalsky wrote…

remove (debug) prints

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

Review status: 0 of 12 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


sandbox_scripts/environment/setup/setup_resources.py, line 10 at r1 (raw file):

Previously, jimbr70 wrote…

change made

Done.


sandbox_scripts/environment/setup/setup_resources.py, line 77 at r1 (raw file):

Previously, kalsky wrote…

please remove these lines from the setup_resources file, you can add them in a test file that later will call/use this class.

Done.


Comments from Reviewable

@kalsky
Copy link
Member

kalsky commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 140 at r1 (raw file):

Previously, jimbr70 wrote…

Done.

can't see that it's done


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 132 at r1 (raw file):

Previously, kalsky wrote…

can't see it that it's done

Done.


Comments from Reviewable

@jimbr70
Copy link
Author

jimbr70 commented Apr 20, 2017

sandbox_scripts/QualiEnvironmentUtils/Resource.py, line 140 at r1 (raw file):

Previously, kalsky wrote…

can't see that it's done

Done.


Comments from Reviewable

@kalsky
Copy link
Member

kalsky commented Apr 20, 2017

Reviewed 4 of 10 files at r1, 6 of 8 files at r2, 4 of 4 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.


Comments from Reviewable

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.03%) to 26.203% when pulling 70ccfd3 on refactor-saveNrestore-GITLAB into a07a839 on refactor_saveNrestore.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 26.413% when pulling 05d21fc on refactor-saveNrestore-GITLAB into a07a839 on refactor_saveNrestore.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 26.476% when pulling af92e24 on refactor-saveNrestore-GITLAB into a07a839 on refactor_saveNrestore.

@kalsky
Copy link
Member

kalsky commented Apr 21, 2017

Reviewed 1 of 3 files at r4, 4 of 4 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@kalsky kalsky merged commit 4d16b5a into refactor_saveNrestore Apr 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants