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

Fix for issue #995 #1147

Closed
wants to merge 12 commits into from
Closed

Fix for issue #995 #1147

wants to merge 12 commits into from

Conversation

chidanandpujar
Copy link
Collaborator

Fix for issue jnpr.junos.utils.start_shell option to choose shell (sh or csh) #995
with this fix StartShell supports the argument shell_type for on-box , we can pass csh or sh shell types

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 93.38% when pulling d7340e8 on chidanandpujar:py_junos_eznc_start_shell_type into fd81d47 on Juniper:master.

@coveralls
Copy link

coveralls commented Jan 3, 2022

Coverage Status

Coverage decreased (-0.1%) to 93.376% when pulling c713eca on chidanandpujar:py_junos_eznc_start_shell_type into fd81d47 on Juniper:master.

)
else:
self._client = open_ssh_client(dev=self._nc)
self._chan = self._client.invoke_shell()

got = self.wait_for(r"(%|>|#|\$)")
if got[-1].endswith(_JUNOS_PROMPT):
self.send("start shell")
self.send("start shell self.shell_type")
Copy link
Collaborator

@dineshbaburam91 dineshbaburam91 Jan 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to concatenate two strings

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review comments ,
Fixed the command
self.send("start shell " + self.shell_type)

shell=False,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
close_fds=1,
Copy link
Collaborator

@dineshbaburam91 dineshbaburam91 Jan 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you create fd and bufsize?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the close_fds and bufsize
self._chan = subprocess.Popen(
["cli", "start", "shell", self.shell_type],
shell=False,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
)

@@ -100,18 +112,20 @@ def open(self):
"""
if self.ON_JUNOS is True:
self._chan = subprocess.Popen(
["cli", "start", "shell"],
["cli", "start", "shell", self.shell_type],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behaviour if start shell is None

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shell_type="csh" is initialized with csh by default in init of StartShell,

@dineshbaburam91
Copy link
Collaborator

Please update UT.

@chidanandpujar
Copy link
Collaborator Author

Closing this pull request and submitting new one .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants