Arvados

In meeting today we discussed that letting all prefixed properties through in the Go SDK should be fine. Strictness is meant to help guide people toward keeping data normalized, not to absolutely lock people out of adding unsupported metadata.

arv: prefix is the frontrunner. We can continue supporting the current un-prefixed names just as historical exceptions.

I have 19980-arv-prefix-properties up at f41bb69fdd60627f6e4880f51ed48f69e79af760

I can't get the tests to pass because rails_restart_test.go keeps failing. Most recent failure is developer-run-tests: #3503

I even took a side track to fix what I believe is a real bug that might be responsible for this failure, or at least one possible cause of it. That fix is included in the branch. Jenkins keeps failing, every time.

The tests pass on my machine. The changes in the branch aren't anything that I can imagine being environment-specific. Even assuming my commit doesn't actually fix whatever bug is causing the test to fail, having like five different test runs fail all to the same race condition would have to be incredible bad luck.

Going to need help getting unblocked with this.

Brett Smith wrote in #note-7:

I have 19980-arv-prefix-properties up at f41bb69fdd60627f6e4880f51ed48f69e79af760

Successful build at developer-run-tests: #3504 . I don't know why it suddenly worked but whatever.

The story talks about accepting the arv: prefix, but I didn't want to implement that with strings.HasPrefix because I don't think we want to accept the literal key arv:. So to deal with that, and avoid naming shenanigans in general, I semi-arbitrarily added the rule that the first character after the colon needs to be alphabetic.

That covers all the git* properties we currently use. If we wanted to relax that rule (letting more characters be first) or expand it (limiting all the characters in the name, not just the first), I think that wouldn't be a big deal to do in the future, and I would be fine with it (within reason).

Ready for review.

Lgtm!

Brett Smith wrote in #note-8:

The story talks about accepting the arv: prefix, but I didn't want to implement that with strings.HasPrefix because I don't think we want to accept the literal key arv:. So to deal with that, and avoid naming shenanigans in general, I semi-arbitrarily added the rule that the first character after the colon needs to be alphabetic.

Talked about this with Tom some in 1:1. He was okay with accepting the literal arv:. And in general there was a feeling of, this doesn't actually enforce good names, it just creates a weird divide of names that are reserved by the system, and names that look like they should be reserved by the system but aren't really. So now it is implemented with strings.HasPrefix, in 3429718113f11e999dd745e94ff981a4f3b01c57.

IMO it would be worth considering a story about putting some limits on property names in general, to prevent people from trying to screw with other users or components by setting bizarre names. But that's a separate story that should apply to all property names, not just system ones.