-
Notifications
You must be signed in to change notification settings - Fork 11
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
Enable scaffolding of cliconf and terminal plugin files #16
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: GomathiselviS <[email protected]>
cc @NilashishC |
@@ -0,0 +1,206 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of the code in this template is platform specific. IMO, we should only be adding methods that are marked as "abstractmethods" in CliconfBase and possibly keep a docstring and an empty definition to allow end users to understand what that method is meant to be and how they should implement it.
|
||
def on_open_shell(self): | ||
try: | ||
for cmd in (b"terminal length 0", b"terminal width 512"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length and width values are not same across all platforms. Maybe we can specify what this method does in the method docstring and let the collection maintainers add the correct values.
def __init__(self, *args, **kwargs): | ||
super(Cliconf, self).__init__(*args, **kwargs) | ||
|
||
@enable_mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enable
privilege escalation is not valid across all platforms. Maybe we can skip this?
import time | ||
import re | ||
|
||
from ansible.errors import AnsibleConnectionFailure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CliconfBase has multiple abstractmethods
defined. An subclass needs to implement all that else the cliconf object is not created. Can we add stubs for all those methods here?
Signed-off-by: GomathiselviS [email protected]
Fixes #11
This PR enables the scaffolding of cliconf and terminal plugin files via cli_rm_builder.