Daisy

Merge lp://staging/~ev/daisy/core-storage-providers into lp://staging/daisy

core-storage-providers
Merge into trunk

Proposed by Evan on 2013-03-04

Status:	Merged
Merged at revision:	241
Proposed branch:	lp://staging/~ev/daisy/core-storage-providers
Merge into:	lp://staging/daisy
Diff against target:	627 lines (+393/-86) 5 files modified daisy/configuration.py (+46/-7) daisy/retracer.py (+77/-29) daisy/submit_core.py (+161/-46) test/test_core_providers.py (+70/-0) test/test_submit.py (+39/-4)
To merge this branch:	bzr merge lp://staging/~ev/daisy/core-storage-providers
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
JuanJo Ciarlante (community)	2013-03-04	Needs Fixing on 2013-03-06
Brian Murray (community)	2013-03-04	Approve on 2013-03-05
Review via email: mp+151444@code.staging.launchpad.net

lp://staging/~ev/daisy/core-storage-providers updated on 2013-03-05

239. By Evan on 2013-03-04: Merge in my branch that removes the hardcoding of localhost and also fixes references to the now non-existent Buckets CF (replaced by Bucket).
240. By Evan on 2013-03-04: Fix core submission test. It was setting the configuration after it was already transformed by validate_configuration.
241. By Evan on 2013-03-04: Add missing core providers test.
242. By Evan on 2013-03-05: Loosely test writing to S3 by mocking out much of it. Use the stream writing capabilities of boto.

Revision history for this message

Brian Murray (brian-murray) wrote on 2013-03-05:

This looks good to me although I am curious about a couple of nitpicky things.

1) test_core_providers.py has a commented out import '+#from daisy import retracer'

2) there also seems to be a print (print s3con.call_args) in test_submit.py

review: Approve

Revision history for this message

JuanJo Ciarlante (jjo) wrote on 2013-03-06:

Looks great really, tnx for it.

A fix , and a couple of nits (as per IRC chatter)
#1 we need to join provider_data['path'] also at retracer.py
#2 lets s/nfs/local/ everywhere, likewise s/write_to_san/write_to_local/
#3 please decouple the new CF usage ('Bucket') to another MP, ie let's keep using 'Buckets' meanwhile

review: Needs Fixing

lp://staging/~ev/daisy/core-storage-providers updated on 2013-03-06

243. By Evan on 2013-03-06: Use the path member and send the uuid:nfs rather than continuing to pass the path for nfs backed core files, matching the behaviour for other providers.
244. By Evan on 2013-03-06: Rename the nfs core storage provider to local, as suggested by jjo.
245. By Evan on 2013-03-06: s/write_to_san/write_to_local/g

Revision history for this message

Evan (ev) wrote on 2013-03-06:

Fixed jjo's comments:

#1 didn't require a test change, since the code functioned correctly. The correct path was written to, but we were using the path as part of the key on Rabbit. This was inconsistent with the other providers, so I changed it to fetch the path from the provider information rather than splitting it off the Rabbit message.

#2 is fixed in r243, 244, and 245.

#3 was fixed by merging lp:~ev/daisy/hardcoded_localhost into lp:daisy.

lp://staging/~ev/daisy/core-storage-providers updated on 2013-03-06