Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
Lucas Di Pentima
@ldipenti
Have a great holiday!
Lucas Di Pentima
@ldipenti
:+1:
Lucas Di Pentima
@ldipenti
@DannyNs 18169 LGTM, thanks!
Lucas Di Pentima
@ldipenti
@tomclegg Do you know if we’re explicitly supporting named pipes use on arv-put? (This is re: 18480)
Tom Clegg
@tomclegg
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 :)
Lucas Di Pentima
@ldipenti
Yeah, I would say that if it isn’t a currently usage pattern, let’s not enable it :P
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
Tom Clegg
@tomclegg
Yeah. I added a note to the ticket to vote for it.
Lucas Di Pentima
@ldipenti
Thanks!
Daniel Kutyła
@DannyNs
thank you
Lucas Di Pentima
@ldipenti
@tomclegg https://dev.arvados.org/issues/18480#note-3 is ready for review, in case you have time.
Lucas Di Pentima
@ldipenti
@tomclegg whoops, forgot to add the named pipe to the symlink test dir, it fails as you predicted. Let me adjust the branch a little bit
Lucas Di Pentima
@ldipenti
@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
Tom Clegg
@tomclegg
@ldipenti 18480 LGTM
Lucas Di Pentima
@ldipenti
Thanks!
Tom Clegg
@tomclegg
Lucas Di Pentima
@ldipenti
Awesome, thanks!
Daniel Kutyła
@DannyNs
Lucas Di Pentima
@ldipenti
Ok, will do
Lucas Di Pentima
@ldipenti
@DannyNs 18482 LGTM, thanks!
Lucas Di Pentima
@ldipenti

@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?

Tom Clegg
@tomclegg
hm, maybe the Go SSH server has a newer-version SSH handshake protocol or something?
Lucas Di Pentima
@ldipenti
Does the ssh server has anything to do with golang.org/x/crypto? Because I’m not seeing anything named ‘ssh’ upgraded.
Tom Clegg
@tomclegg
yes, ssh is in that x/crypto repo
Daniel Kutyła
@DannyNs
@ldipenti thank you
Lucas Di Pentima
@ldipenti
Good morning. WB2 branch looking for a reviewer: https://dev.arvados.org/issues/18484#note-5
Lucas Di Pentima
@ldipenti
@tomclegg Resuming work on 18491: Is that test failure a sign that the test could be improved by not testing something that’s version dependant?
Tom Clegg
@tomclegg
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?
Lucas Di Pentima
@ldipenti
Sounds good, thanks @tomclegg
Lucas Di Pentima
@ldipenti
@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)
Tom Clegg
@tomclegg
@ldipenti yes, LGTM thanks!
Lucas Di Pentima
@ldipenti
Great, thanks!
Peter Amstutz
@tetron
Lucas Di Pentima
@ldipenti
Lucas Di Pentima
@ldipenti
@stephen304 Are you aching to do some wb2 review? I have this branch ready https://dev.arvados.org/issues/18484#note-5 :)
Stephen Smith
@stephen304
@ldipenti I can get to that in a bit
Lucas Di Pentima
@ldipenti
Thanks!
Lucas Di Pentima
@ldipenti
@stephen304 Another one ready for review https://dev.arvados.org/issues/18257#note-10
Stephen Smith
@stephen304
Taking a look now
Stephen Smith
@stephen304
Lucas Di Pentima
@ldipenti
great, thanks!
Stephen Smith
@stephen304
Lucas Di Pentima
@ldipenti
Thanks! merging
Stephen Smith
@stephen304
@tetron https://dev.arvados.org/issues/18123#note-13 is ready for review
Peter Amstutz
@tetron:matrix.org
[m]
awesome!