Feature #4717

[API] [SDKs] Read-only flag on keep_services table as hint to clients not to try to write.

Added by Peter Amstutz over 5 years ago. Updated about 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
API
Target version:
Start date:
05/12/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Subtasks

Task #6040: Review branch: 4717-read-only-keep-services-flagResolvedRadhika Chippada

Task #5634: Update Go SDKResolvedRadhika Chippada

Task #5633: Update Python SDKResolvedRadhika Chippada

Task #5632: Update API serverResolvedRadhika Chippada


Related issues

Related to Arvados - Feature #4716: [Keep] Keepstore has read-only mode (deny all PUT or DELETE)Rejected

Related to Arvados - Feature #5414: [Keep] Use data stored remotely (not in local keepstore)Closed03/29/2015

Associated revisions

Revision 4110970a
Added by Radhika Chippada about 5 years ago

closes #4717
Merge branch '4717-read-only-keep-services-flag'

History

#1 Updated by Tom Clegg over 5 years ago

  • Target version deleted (Arvados Future Sprints)

#2 Updated by Brett Smith over 5 years ago

  • Category set to API

#3 Updated by Tom Clegg over 5 years ago

  • Target version set to Arvados Future Sprints

#4 Updated by Tom Clegg over 5 years ago

  • Subject changed from [API] Read-only flag on keep_services table as hint to clients not to try to write. to [API] [SDKs] Read-only flag on keep_services table as hint to clients not to try to write.

#5 Updated by Tom Clegg over 5 years ago

  • Story points set to 1.0

#6 Updated by Tom Clegg over 5 years ago

  • Target version changed from Arvados Future Sprints to 2015-04-29 sprint

#7 Updated by Tom Clegg over 5 years ago

  • Assigned To set to Tom Clegg
  • Story points changed from 1.0 to 0.5

#8 Updated by Tom Clegg over 5 years ago

  • Story points changed from 0.5 to 1.0

#9 Updated by Tom Clegg over 5 years ago

Noted #5414 as a "check for conflicts if 5414 isn't merged yet" reminder.

#10 Updated by Peter Amstutz over 5 years ago

  • Assigned To changed from Tom Clegg to Peter Amstutz

#11 Updated by Tom Clegg over 5 years ago

  • Target version changed from 2015-04-29 sprint to 2015-05-20 sprint

#12 Updated by Radhika Chippada over 5 years ago

  • Status changed from New to In Progress
  • Assigned To changed from Peter Amstutz to Radhika Chippada

#13 Updated by Tom Clegg about 5 years ago

At e41260f

In the Go SDK, WritableRoots and writableRoots seem to mean writableLocalRoots, so should probably be renamed accordingly.

For style and readability, suggest collapsing the types, putting the locals together, and keeping the newX parameter name trend:

-func (kc *KeepClient) SetServiceRoots(newLocals, newGateways map[string]string, writableRoots map[string]string)
+func (kc *KeepClient) SetServiceRoots(newLocals, newWritableLocals, newGateways map[string]string)

pull worker doesn't write to its keepClient, so suggest this in pull_worker.go:

-       keepClient.SetServiceRoots(service_roots, nil, keepClient.WritableRoots())
+       keepClient.SetServiceRoots(service_roots, nil, nil)

Please use go test -check.vv instead of cluttering tests with debug printfs:

log.Printf("TestPutWant2ReplicasWithOnlyOneWritableRoots")

In keep.py, suggest:

-        if (need_writable == True):
+        if need_writable:

And:

-                self._writable_services = [{
-                    'uuid': 'proxy',
-                    '_service_root': proxy,
-                    }]
+                self._writable_services = self._keep_services

Migration: I think you could change "def up" to "def change" and remove the explicit down-migration, and Rails would DTRT. (But then, I also thought "rails g migration" would have done this for you, but maybe not...?)

#14 Updated by Radhika Chippada about 5 years ago

Addressed all comments and merged into master. (Yes, the change method worked for both up and down for this case). Thanks.

#15 Updated by Radhika Chippada about 5 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:4110970a34689ed526c3365e9b64a784fba7efda.

Also available in: Atom PDF