tahoe-lafs/tahoe-lafs
tahoe-lafs
/
tahoe-lafs
2
0
Fork 0
Code Issues Pull Requests 23 Packages Projects Releases 1 Wiki Activity

exit on stdin close #1212

Merged
meejah merged 15 commits from 3921.exit-on-stdin-close into master 2022-12-02 09:04:46 +00:00
Conversation 5 Commits 15 Files Changed 5 +120 -5
meejah commented 2022-09-01 23:45:07 +00:00 (Migrated from github.com)
Copy Link

Fixes🎫3921

https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3921#ticket

Fixes:ticket:3921 https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3921#ticket
meejah commented 2022-09-08 23:37:02 +00:00 (Migrated from github.com)
Copy Link

Anyone know what's up on the Windows builders? The coverage job seems to succeed (and maybe there's some sort of error trying to print the report?)

Anyone know what's up on the Windows builders? The coverage job seems to succeed (and maybe there's some sort of error trying to print the report?)
exarkun commented 2022-09-09 01:49:40 +00:00 (Migrated from github.com)
Copy Link

The test run includes this output:

2022-09-08T22:55:27.7618842Z   File "C:\hostedtoolcache\windows\Python\3.7.9\x64\lib\runpy.py", line 193, in _run_module_as_main
2022-09-08T22:55:27.7619965Z     "__main__", mod_spec)
2022-09-08T22:55:27.7620724Z   File "C:\hostedtoolcache\windows\Python\3.7.9\x64\lib\runpy.py", line 85, in _run_code
2022-09-08T22:55:27.7622159Z     exec(code, run_globals)
2022-09-08T22:55:27.7622841Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\Scripts\coverage.EXE\__main__.py", line 7, in <module>
2022-09-08T22:55:27.7623581Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\cmdline.py", line 871, in main
2022-09-08T22:55:27.7624091Z     status = CoverageScript().command_line(argv)
2022-09-08T22:55:27.7624734Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\cmdline.py", line 621, in command_line
2022-09-08T22:55:27.7625193Z     **report_args
2022-09-08T22:55:27.7625778Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\control.py", line 913, in report
2022-09-08T22:55:27.7626279Z     return reporter.report(morfs, outfile=file)
2022-09-08T22:55:27.7626921Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\summary.py", line 110, in report
2022-09-08T22:55:27.7627487Z     self.writeout(line[0])
2022-09-08T22:55:27.7628082Z   File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\summary.py", line 32, in writeout
2022-09-08T22:55:27.7628575Z     self.outfile.write(line.rstrip())
2022-09-08T22:55:27.7628942Z OSError: [Errno 28] No space left on device
2022-09-08T22:55:27.7986082Z ##[error]ERROR: InvocationError for command 'D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\Scripts\coverage.EXE' report (exited with code 1)

That seems remarkably unfortunate...

The test run includes this output: ``` 2022-09-08T22:55:27.7618842Z File "C:\hostedtoolcache\windows\Python\3.7.9\x64\lib\runpy.py", line 193, in _run_module_as_main 2022-09-08T22:55:27.7619965Z "__main__", mod_spec) 2022-09-08T22:55:27.7620724Z File "C:\hostedtoolcache\windows\Python\3.7.9\x64\lib\runpy.py", line 85, in _run_code 2022-09-08T22:55:27.7622159Z exec(code, run_globals) 2022-09-08T22:55:27.7622841Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\Scripts\coverage.EXE\__main__.py", line 7, in <module> 2022-09-08T22:55:27.7623581Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\cmdline.py", line 871, in main 2022-09-08T22:55:27.7624091Z status = CoverageScript().command_line(argv) 2022-09-08T22:55:27.7624734Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\cmdline.py", line 621, in command_line 2022-09-08T22:55:27.7625193Z **report_args 2022-09-08T22:55:27.7625778Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\control.py", line 913, in report 2022-09-08T22:55:27.7626279Z return reporter.report(morfs, outfile=file) 2022-09-08T22:55:27.7626921Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\summary.py", line 110, in report 2022-09-08T22:55:27.7627487Z self.writeout(line[0]) 2022-09-08T22:55:27.7628082Z File "D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\lib\site-packages\coverage\summary.py", line 32, in writeout 2022-09-08T22:55:27.7628575Z self.outfile.write(line.rstrip()) 2022-09-08T22:55:27.7628942Z OSError: [Errno 28] No space left on device 2022-09-08T22:55:27.7986082Z ##[error]ERROR: InvocationError for command 'D:\a\tahoe-lafs\tahoe-lafs\.tox\py37-coverage\Scripts\coverage.EXE' report (exited with code 1) ``` That seems remarkably unfortunate...
meejah commented 2022-09-09 05:32:13 +00:00 (Migrated from github.com)
Copy Link

Oh, I didn't see the "out of space" ... looking at all of them, 3.7 and 3.8 have "out of space" and 3.9 and 3.10 fail during writing the report (but don't have an "out of space" exception ... although perhaps its the same reason?)

I guess on the "good news" front, all the tests themselves appear to pass :/

Oh, I didn't see the "out of space" ... looking at all of them, 3.7 and 3.8 have "out of space" and 3.9 and 3.10 fail during writing the report (but don't have an "out of space" exception ... although perhaps its the same reason?) I guess on the "good news" front, all the _tests_ themselves appear to pass :/
exarkun (Migrated from github.com) approved these changes 2022-11-29 13:47:45 +00:00
exarkun (Migrated from github.com) left a comment
Copy Link

Thanks. Some comments inline. Please address and merge.

Thanks. Some comments inline. Please address and merge.
newsfragments/3921.feature
exarkun (Migrated from github.com) commented 2022-11-08 15:47:11 +00:00
Copy Link
`tahoe run ...` will now exit when its stdin is closed.
```suggestion `tahoe run ...` will now exit when its stdin is closed. ```
src/allmydata/scripts/tahoe_run.py
@ -200,4 +209,3 @@
return d
from twisted.internet import reactor
reactor.callWhenRunning(start)
exarkun (Migrated from github.com) commented 2022-11-23 12:41:06 +00:00
Copy Link

Let's catch and ignore ReactorNotRunning but log other stuff? It's always sad when this kind of handling hides a SyntaxError or a NameError or some other simple programming mistake.

Let's catch and ignore ReactorNotRunning but log other stuff? It's always sad when this kind of handling hides a SyntaxError or a NameError or some other simple programming mistake.
src/allmydata/scripts/tahoe_run.py
exarkun (Migrated from github.com) commented 2022-11-23 12:41:26 +00:00
Copy Link

only for Windows testing purposes?

only for Windows testing purposes?
src/allmydata/test/test_runner.py
exarkun (Migrated from github.com) commented 2022-11-08 16:00:31 +00:00
Copy Link
        An exception from our on-close function is discarded.
```suggestion An exception from our on-close function is discarded. ```
exarkun (Migrated from github.com) commented 2022-11-29 13:33:17 +00:00
Copy Link

It returns the StandardIO so I'd call this transport probably.

It returns the `StandardIO` so I'd call this `transport` probably.
exarkun (Migrated from github.com) commented 2022-11-29 13:41:55 +00:00
Copy Link

This was pretty confusing until I recognized that proto is a StandardIO and writeConnectionLost / readConnectionLost here don't refer to IHalfCloseableProtocol but just happen to be the names of the Windows StandardIO implementation details for stdin/stdout closed notification. :/

Too bad Twisted doesn't provide a usable StandardIO test double, I guess? Maybe we can ask for one.

Since lines 649-658 are duplicated in the next test, I'd at least pull them out into a shared helper function.

This was pretty confusing until I recognized that `proto` is a `StandardIO` and `writeConnectionLost` / `readConnectionLost` here don't refer to `IHalfCloseableProtocol` but just happen to be the names of the Windows `StandardIO` implementation details for stdin/stdout closed notification. :/ Too bad Twisted doesn't provide a usable `StandardIO` test double, I guess? Maybe we can ask for one. Since lines 649-658 are duplicated in the next test, I'd at least pull them out into a shared helper function.
meejah (Migrated from github.com) reviewed 2022-12-02 00:03:18 +00:00
src/allmydata/scripts/tahoe_run.py
meejah (Migrated from github.com) commented 2022-12-02 00:03:17 +00:00
Copy Link

yes .. because we need to call .writeConnectionLost() by hand there Because Reasons.

yes .. because we need to call `.writeConnectionLost()` by hand there Because Reasons.
meejah commented 2022-12-02 03:52:25 +00:00 (Migrated from github.com)
Copy Link

Hrrmmm ... two "dead reference errors" and two "disk full" on windows :/

Hrrmmm ... two "dead reference errors" and two "disk full" on windows :/
coveralls commented 2022-12-02 07:37:14 +00:00 (Migrated from github.com)
Copy Link

Coverage Status

Coverage decreased (-0.003%) to 94.692% when pulling 3d831f653b on meejah:3921.exit-on-stdin-close into 7cc023a590 on tahoe-lafs:master.

[![Coverage Status](https://coveralls.io/builds/54777969/badge)](https://coveralls.io/builds/54777969) Coverage decreased (-0.003%) to 94.692% when pulling **3d831f653ba20286ea94e512cf0e87882cbc9e26 on meejah:3921.exit-on-stdin-close** into **7cc023a590f97fb0bcccecda69525558241d0d2d on tahoe-lafs:master**.
Sign in to join this conversation.
Reviewers
No reviewers
exarkun
Labels
Clear labels   
Benchmarking and Performance

   
HTTP Storage Protocol

   
Nevow Removal

   
Python 3 Porting

   
not-for-merge
No Label
Benchmarking and Performance
HTTP Storage Protocol
Nevow Removal
Python 3 Porting
not-for-merge
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/tahoe-lafs#1212
No description provided.
Delete Branch "3921.exit-on-stdin-close"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?