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.

Sign up for GitHub

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

build-server shutdown workaround #1346

Merged
merged 1 commit into from
Jun 5, 2020
Merged

build-server shutdown workaround #1346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/scenarios/shared/sdk_iteration_setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import os
import shutil
from shared import const
from shared import const, util
from dotnet import shutdown_server
from performance.logger import setup_loggers
from logging import getLogger
from argparse import ArgumentParser


SETUP_BUILD = 'setup_build'
SETUP_NEW = 'setup_new'
CLEANUP = 'cleanup'
Expand All @@ -19,7 +20,7 @@ def main():
setup_loggers(True)

if args.operation == SETUP_BUILD:
shutdown_server(verbose=True)
shutdown_dotnet_servers()
if not os.path.isdir(const.TMPDIR):
if not os.path.isdir(const.APPDIR):
raise Exception("\'app\' folder should exist. Please run pre.py.")
Expand All @@ -42,5 +43,13 @@ def main():
shutil.rmtree(const.APPDIR)


def shutdown_dotnet_servers():
# shutdown_server(verbose=True) # This is the correct way to shut down dotnet build servers, but it has been disabled due to https://github.com/dotnet/sdk/issues/10573
getLogger().info("Shutting down dotnet build servers...")
if util.iswin():
os.system('TASKKILL /F /T /IM dotnet.exe || TASKKILL /F /T /IM VSTest.Console.exe || TASKKILL /F /T /IM msbuild.exe')
Copy link
Member

Choose a reason for hiding this comment

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

The || syntax means to only run the commands after it if the first one fails. Is that the behavior that we want?

Copy link
Contributor Author

@ooooolivia ooooolivia Jun 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This command is suggested by @wli3 . I suppose the rationale here is dotnet.exe could be the parent process that spawns the other two, so if dotnet.exe has terminated we'd like to kill the child processes that are still running. Otherwise by specifying /T will be sufficient to kill the parent and child processes together.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

else:
os.system('killall -9 dotnet || killall -9 VSTest.Console || killall -9 msbuild')

if __name__ == "__main__":
main()