Development discussion & coordination for the Arvados project. For Q&A, support and general discussion see https://gitter.im/arvados/community
@ldipenti https://dev.arvados.org/issues/18169?next_issue_id=18168&prev_issue_id=18170#note-17 please review
Not that I can recall. That patch looks like it will inadvertently skip symlinks to directories even when follow_links is enabled, need to check isdir() as well. But skipping block/char devices and fifos seems like an improvement. I imagine in any case where someone truly wants to copy from a fifo to keep, it would be much more predictable/reasonable to use an explicit shell redirect and tell arv-put to copy stdin.
Trying to imagine setting up a directory with multiple fifos and handing it to arv-put and expecting anything reasonable to happen... sounds a little bit crazy
Even before contemplating "resume" behavior :)
Reading from stdin is already available
I was asking because I’m planning to do a test against this, and it might be a little difficult so I wanted to confirm before making the effort
@tomclegg No, it didn’t fail as predicted, I was just doing the wrong test. I’ve enhanced the new test so that it confirms that symlinked directories aren’t ignored: https://dev.arvados.org/issues/18480#note-4
@tomclegg I’m getting this test failure after upgrading a go module and running go mod tidy:
16:50:57 ----------------------------------------------------------------------
16:50:57 FAIL: container_gateway_test.go:189: ContainerGatewaySuite.TestConnect
16:50:57
16:50:57 connecting to localhost:39667
16:50:57 container_gateway_test.go:213:
16:50:57 c.Check(buf[:4], check.DeepEquals, []byte{0, 0, 1, 0xfc})
16:50:57 ... obtained []uint8 = []byte{0x0, 0x0, 0x2, 0xc}
16:50:57 ... expected []uint8 = []byte{0x0, 0x0, 0x1, 0xfc}https://dev.arvados.org/issues/18491#note-2
Does it give any clue on what could be the problem?
Yes, the intent is to check that we have bidirectional communication with an SSH service. Scanning the RFC now, I think the first 4 bytes are the length of the first data packet the server is sending, which would mean the newer version is sending 16 more bytes in that first packet, which seems unsurprising, and yes, unnecessarily fragile...
I think we could just remove that check -- the previous line already confirms that we can read 4 bytes, so things are already looking good. Maybe just c.Log() the bytes for fun and leave it at that?
@tomclegg https://dev.arvados.org/issues/18491#note-5 is ready for review, in case you have a couple of minutes (It’s just a module upgrade + the test adjustment)
@stephen304 Are you aching to do some wb2 review? I have this branch ready https://dev.arvados.org/issues/18484#note-5 :)
