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

Run CI-testing on GHA/Windows #337

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

amotl
Copy link
Contributor

@amotl amotl commented Nov 13, 2020

Dear Mathias,

while working on crate/crate-python#388, we faced some deadlock issues when tearing down the testlayer on GHA/Windows. So, we wanted to take the chance to also test the testlayer incorporated into cr8.

Please consider this as a work in progress / draft unless further notice.

With kind regards,
Andreas.

@amotl amotl force-pushed the amo/windows branch 2 times, most recently from 1e15689 to 20b6b8f Compare November 13, 2020 10:19
@amotl
Copy link
Contributor Author

amotl commented Nov 13, 2020

Apparently, the deadlock issues on testlayer teardown also happen here.

======================================================================
ERROR: D:\a\cr8\cr8\tests\..\README.rst
Doctest: README.rst
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\doctest.py", line 2178, in tearDown
    self._dt_tearDown(test)
  File "D:\a\cr8\cr8\tests\test_integration.py", line 38, in teardown
    node.stop()
  File "D:\a\cr8\cr8\cr8\run_crate.py", line 395, in stop
    self.process.communicate(timeout=10)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\subprocess.py", line 1024, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "C:\hostedtoolcache\windows\Python\3.8.6\x64\lib\subprocess.py", line 1397, in _communicate
    raise TimeoutExpired(self.args, orig_timeout)
subprocess.TimeoutExpired: Command '['C:\\Users\\runneradmin\\.cache\\cr8\\crates\\crate-4.3.1\\bin\\crate.bat', '-Cdiscovery.initial_state_timeout=0', '-Cnetwork.host=127.0.0.1', '-Cudc.enabled=false', '-Ccluster.name=cr8-tests', '-Chttp.port=44200-44250', '-Cpath.data=C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp2yvi7ihz']' timed out after 10 seconds

-- https://github.com/mfussenegger/cr8/pull/337/checks?check_run_id=1395324258#step:6:237

@amotl
Copy link
Contributor Author

amotl commented Nov 13, 2020

Another thing I would like to share in this context is that

echo "select name from sys.cluster" | cr8 timeit --hosts http://127.0.0.1:44200

will croak with

SqlException: SQLParseException[line 1:1: mismatched input '"select name from sys.cluster"' expecting {'SELECT', 'DEALLOCATE', 'CREATE', 'ALTER', 'KILL', 'BEGIN', 'COMMIT', 'ANALYZE', 'DISCARD', 'EXPLAIN', 'SHOW', 'OPTIMIZE', 'REFRESH', 'RESTORE', 'DROP', 'INSERT', 'VALUES', 'DELETE', 'UPDATE', 'SET', 'RESET', 'COPY', 'GRANT', 'DENY', 'REVOKE'}] occurred using: {"stmt": "\"select name from sys.cluster\""}

-- https://github.com/mfussenegger/cr8/pull/337/checks?check_run_id=1395743511#step:6:173

This happens even when running that on bash after 94ad6b2.

Apparently, the quotes are taken verbatim when echoing a string through a pipe. I believe [1] is discussing this very issue and others are experiencing similar pains, see also rundeck/rundeck#602 and rundeck/rundeck#1406.

[1] https://stackoverflow.com/questions/804646/how-do-you-strip-quotes-out-of-an-echoed-string-in-a-windows-batch-file

@amotl
Copy link
Contributor Author

amotl commented Dec 17, 2020

We have been able to dedicate some more minutes of research into the deadlocking issue on layer teardown when running on Windows.

Firstly, we have been able to discover very valuable resources at [1,2,3], which give many general insights into signal handling for child processes on Windows.

However, in the context of subprocess.Popen, we also just found [4,5,6], where [5] specifically states that the CREATE_NEW_PROCESS_GROUP creationflags parameter to subprocess.Popen is required for using os.kill() on the subprocess. We will try that within one of the next iterations.

[1] https://stackoverflow.com/questions/813086/can-i-send-a-ctrl-c-sigint-to-an-application-on-windows
[2] https://stackoverflow.com/questions/3342941/kill-child-process-when-parent-process-is-killed/4657392#4657392
[3] https://stanislavs.org/stopping-command-line-applications-programatically-with-ctrl-c-events-from-net/
[4] https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
[5] https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
[6] https://docs.python.org/3/library/asyncio-subprocess.html#asyncio.asyncio.subprocess.Process.send_signal

UnicodeEncodeError: 'charmap' codec can't encode character '\u2192' in position 65: character maps to <undefined>
@amotl amotl force-pushed the amo/windows branch 2 times, most recently from ce7399e to 9e774b0 Compare February 10, 2024 23:47
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.

1 participant