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

Windows support for Service tool added. #3573

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SRIKKANTH
Copy link
Collaborator

@SRIKKANTH SRIKKANTH commented Dec 30, 2024

This pull request adds Windows support for the Service tool.

@SRIKKANTH SRIKKANTH force-pushed the smyakam/service_tool_29_12_20024 branch 5 times, most recently from 5678bbc to 2f38007 Compare December 30, 2024 20:05
def _check_exists(self) -> bool:
return True

def restart_service(self, name: str, ignore_exit_code: int = 0) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Move public methods above private methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

lisa/schema.py Outdated
@@ -431,6 +431,16 @@ class DiskType(str, Enum):
]


class WindowsServiceStatus(int, Enum):
Copy link
Member

Choose a reason for hiding this comment

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

Move it along in to the service class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

self._log.debug(
f"service '{name}' is not ready yet, retrying... after 3 seconds"
)
sleep(3)
Copy link
Member

Choose a reason for hiding this comment

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

Change the interval to 0.5 second.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

self.wait_for_service_start(name)

def wait_for_service_start(self, name: str) -> None:
for _ in range(10):
Copy link
Member

Choose a reason for hiding this comment

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

Create a builtin Timer by create_timer to count the timeout by seconds, instead of count by times.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

):
return

self._log.debug(
Copy link
Member

Choose a reason for hiding this comment

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

Output before the waiting once, not output during waiting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

)
sleep(3)

raise LisaException(f"service '{name}' failed to start")
Copy link
Member

Choose a reason for hiding this comment

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

Give more details, like wait 30 seconds timeout, it's still in NNN status.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

output_json=True,
fail_on_error=False,
)
if not service_status:
Copy link
Member

Choose a reason for hiding this comment

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

Use above fail_on_error, instead of custom error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed


def wait_for_service_start(self, name: str) -> None:
for _ in range(10):
service_status = self._powershell.run_cmdlet(
Copy link
Member

Choose a reason for hiding this comment

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

Use the _get_status, not to implement it again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

raise LisaException(f"service '{name}' does not exist")
return schema.WindowsServiceStatus(service_status["Status"])

def stop_service(self, name: str) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Wait on stop too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

)
assert_that(self._get_status(name)).described_as(
f"Failed to stop service {name}"
).is_not_equal_to(schema.WindowsServiceStatus.RUNNING)
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't make sense, why stop service expect the service status is running?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

).is_not_equal_to(schema.WindowsServiceStatus.RUNNING)

def enable_service(self, name: str) -> None:
raise NotImplementedError()
Copy link
Member

Choose a reason for hiding this comment

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

use pass instead of raise exception.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

raise NotImplementedError()


class WindowsService(Tool):
Copy link
Member

Choose a reason for hiding this comment

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

Move Windows class to the end like other tools.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@SRIKKANTH SRIKKANTH force-pushed the smyakam/service_tool_29_12_20024 branch from 9cfc7ab to 727572d Compare January 1, 2025 08:18
@SRIKKANTH SRIKKANTH marked this pull request as ready for review January 1, 2025 08:23
@SRIKKANTH SRIKKANTH requested a review from LiliDeng as a code owner January 1, 2025 08:23
@SRIKKANTH SRIKKANTH force-pushed the smyakam/service_tool_29_12_20024 branch from 5b555e0 to 72582c5 Compare January 2, 2025 05:53
This pull request adds Windows support for the Service tool.
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.

2 participants