Opened 9 years ago

Closed 6 years ago

Last modified 6 years ago

#1636 closed defect (fixed)

Journal Entry storage / sharing via USB stick doesn't work

Reported by: martin.langhoff Owned by: alsroot
Priority: Unspecified by Maintainer Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Unspecified Keywords: r?
Cc: Distribution/OS: Unspecified
Bug Status: New

Description

Tested on OLPC F11 builds... -- this is also known as http://dev.laptop.org/ticket/9657 -- and there is very good discussion there.

On 0.84.x, if I

1 - using TurtleArt, create a new Journal entry (named "F11-100") with some content (and exit activity)

2 - plug in a USB stick

3 - copy the Journal entry to the USB stick via drag-n-drop in the Journal...

4 - exploring the USB disk shows a F11-100.gtar file with _no_ metadata, and the metadata seems to (maybe) be stored in a database in .olpc.store/index

and then...

5 - unmount, unplug the USB disk

6 - plug it back in, wait for it to mount...

7 - Journal shows F11-100.gtar, and doesn't know what to do with it.

Sugar 0.84 has fixed an old problem (the Journal going to la-la-land if the .olpc.store/inded DB was corrupt) by completely ignoring the metadata DB.

Now, the metadata DB had a role in the workflow I describe above. Has anything taken that role? Maybe the new mechanism isn't kicking in correctly? Does it work on 0.86 somehow? (Maybe we can backport it?)

In the field, exchanging and storing journal entries via USB stick is a major thing.

In the back of my mind, I had expected that Journal Entry Bundles (JEBs) would be used for this, but

the Journal doesn't store a JEB on the disk
the Journal doesn't recognize a valid JEB on the disk
If there is no other fix in sight, the best course of action I can propose is to use JEBs as I describe here.

Attachments (11)

0001-Removable-disk-Save-metadata-and-preview-dlo-9657.patch (2.2 KB) - added by martin.langhoff 9 years ago.
0002-Removable-disk-read-json-formatted-.metadata-and-.pr.patch (4.4 KB) - added by martin.langhoff 9 years ago.
0003-Removable-disk-Handle-renames-dlo-9657.patch (2.8 KB) - added by martin.langhoff 9 years ago.
0004-Removable-disk-delete-preview-and-metadata-dlo-9657.patch (1.2 KB) - added by martin.langhoff 9 years ago.
sugar-1636-pre.patch (6.4 KB) - added by alsroot 9 years ago.
all revert/cherry-picked patches to support original patches
sugar-1636.patch (9.6 KB) - added by alsroot 9 years ago.
patch whch is based on sugar-1636-pre.patch
tmpSVjeVO (11.3 KB) - added by martin.langhoff 9 years ago.
sugar-1636-take2.patch
0001-Feedback-when-deleting-files-on-a-USB-Stick-10351.patch (4.1 KB) - added by erikos 9 years ago.
Feedback-when-deleting-files-on-a-USB-Stick http://dev.laptop.org/ticket/10351
0001-Share-Journal-entries-over-USB-1636.patch (10.0 KB) - added by erikos 9 years ago.
Refactored patch from Martin and addressed the review comments from Tomeu
0001-Journal-entry-sharing.patch (10.7 KB) - added by erikos 8 years ago.
Sharing/Backup Journal entries using a storage device (branch master)
0001-Convert-Journal-entries-that-have-been-saved-to-a-st.patch (6.1 KB) - added by erikos 8 years ago.
Convert Journal entries that have been saved to a storage device in 0.82 (branch master)

Download all attachments as: .zip

Change History (36)

comment:1 Changed 9 years ago by martin.langhoff

In discussion on the dlo bug, JEBs are not really favoured, and I have a patchseries that stores and reads the metadata in ".hidden" files.

Handles creation, renames, deletion, etc.

Changed 9 years ago by martin.langhoff

comment:2 Changed 9 years ago by martin.langhoff

  • Component changed from sugar to journal
  • Owner changed from tomeu to alsroot

The patches are against v0.84 . IIRC, they apply almost perfect on v0.86 .

comment:3 Changed 9 years ago by martin.langhoff

  • Keywords r? added

comment:4 in reply to: ↑ description Changed 9 years ago by alsroot

Replying to martin.langhoff:

Tested on OLPC F11 builds... -- this is also known as http://dev.laptop.org/ticket/9657 -- and there is very good discussion there.

On 0.84.x, if I

1 - using TurtleArt, create a new Journal entry (named "F11-100") with some content (and exit activity)

2 - plug in a USB stick

3 - copy the Journal entry to the USB stick via drag-n-drop in the Journal...

4 - exploring the USB disk shows a F11-100.gtar file with _no_ metadata, and the metadata seems to (maybe) be stored in a database in .olpc.store/index

and then...

5 - unmount, unplug the USB disk

6 - plug it back in, wait for it to mount...

7 - Journal shows F11-100.gtar, and doesn't know what to do with it.

Sugar 0.84 has fixed an old problem (the Journal going to la-la-land if the .olpc.store/inded DB was corrupt) by completely ignoring the metadata DB.

Now, the metadata DB had a role in the workflow I describe above. Has anything taken that role? Maybe the new mechanism isn't kicking in correctly? Does it work on 0.86 somehow? (Maybe we can backport it?)

since 0.84, non-ds sources treated in RO mode, initial 0.84 releases just failed, in 0.86 and last 0.84 patches(afaik) do not fail

For 0.88, was proposed feature
http://wiki.sugarlabs.org/go/Features/Sugar_Bundles

ml discussion
http://thread.gmane.org/gmane.linux.laptop.olpc.sugar/19495/focus=19499

in my mind, having unified sugar bundles much preferable then bunch of special files that accompany Journal objects on USB sources(and not only).

comment:5 Changed 9 years ago by alsroot

btw better to move this discussion to ml thread(I posted before)
since that thread is not so hot, I guess, release manager could suppose it's not useful improvement for next sugar release

comment:6 follow-up: Changed 9 years ago by martin.langhoff

By all means, bring it back to the ml -- I did open a thread on this topic a while ago, and it died. Look for 'User workflow sharing "Journal Entries" over USB sticks'

Some points however:

1 - Any kind of Sugar bundle fails in a major requirement: interop with the rest of the world. As implemented in these patches, a file from Write.xo gets saved on the external media with the conventional extension (rtf? doc?). Image files are saved as PNGs.

This allows editing on a 3rd party machine that may not be running Sugar. PNGs and RTFs are editable just fine in a number of standard non-Sugar editing programs.

This argument convincingly shot down my JEB proposal which is similar to what you propose. The full discussion of various pros and cons at http://dev.laptop.org/ticket/9657

2 - This is an important regression in 0.84 and we (OLPC) cannot hit the field in major deployments like Uruguay where users are _very_ used to this workflow.

Some key workflows/"usage idioms" get quickly ingrained into users. This is one of them.

3 - This is tangled with another related issue: in large deployments like Uy, Sugar 0.84 needs to be able to read USB disks with 0.82 "files". More details at http://dev.laptop.org/ticket/9658 (note! this is different from the other bug linked)

Hope to have the patch for that cooked soon. Will open a ticket for it.

comment:7 in reply to: ↑ 6 Changed 9 years ago by alsroot

Replying to martin.langhoff:

By all means, bring it back to the ml -- I did open a thread on this topic a while ago, and it died. Look for 'User workflow sharing "Journal Entries" over USB sticks'

Some points however:

1 - Any kind of Sugar bundle fails in a major requirement: interop with the rest of the world. As implemented in these patches, a file from Write.xo gets saved on the external media with the conventional extension (rtf? doc?). Image files are saved as PNGs.

This allows editing on a 3rd party machine that may not be running Sugar. PNGs and RTFs are editable just fine in a number of standard non-Sugar editing programs.

This argument convincingly shot down my JEB proposal which is similar to what you propose. The full discussion of various pros and cons at http://dev.laptop.org/ticket/9657

2 - This is an important regression in 0.84 and we (OLPC) cannot hit the field in major deployments like Uruguay where users are _very_ used to this workflow.

Some key workflows/"usage idioms" get quickly ingrained into users. This is one of them.

3 - This is tangled with another related issue: in large deployments like Uy, Sugar 0.84 needs to be able to read USB disks with 0.82 "files". More details at http://dev.laptop.org/ticket/9658 (note! this is different from the other bug linked)

Hope to have the patch for that cooked soon. Will open a ticket for it.

If I correct understand http://wiki.sugarlabs.org/go/Features/Policy and the issue this ticket about, such changes need to be proposed as a feature(on policy terms), code patches is a second phase, it let us weigh all possible ways

e.g. I'm still not sure that bundles don't play here at all:

  • 1st user uploaded journal object to file sharing server
  • 2nd user wants to download it and attach to email
  • 3rd user is going to upload attached object to journal and expect to to see all metadata that 1st user has

we could somehow mix bundles and another methods

Any way having wiki page on http://wiki.sugarlabs.org/go/Features with explanation how someone see this feature would be useful.

comment:8 Changed 9 years ago by martin.langhoff

Aleksey -

this is not a new feature, but a regression.

I agree that with server-mediated cases (which would be a new feature, never implemented before in Sugar) something like a JEB makes more sense (for example, the XS serves JEBs when relevant). I do think JEBs or a unified bundle format has a big role to play.

But that is a separate discussion -- we're dealing with a regression on a widely used functionality.

comment:9 Changed 9 years ago by alsroot

I agree that this is a regression for 0.82 users but the bad thing here is that datastore was changed much in 0.84(so already two stable releases) and in development sense fix for this ticket is a new feature(since all ds code for non-ds sources was removed in 0.84). We can observe this(treating non-ds sources) case once more, accept new scheme and implement fix for this ticket accordingly.

btw maybe I didn't get you correctly, you didn't set milestone field for this ticket, did you have in mind fixing this regression primarily for 0.84? Since we can't implement new features for 0.84, it could be special regression-fix for 0.84 but even in that case having 0.88 discussion and preparing this ticket fix accordingly(to simplify moving to 0.88+) would be useful.

comment:10 Changed 9 years ago by alsroot

  • Milestone changed from Unspecified by Release Team to 0.84
  • Version changed from Unspecified to 0.84.x

So, it was about 0.84
sorry then(I had in mind primarily 0.88)

comment:11 Changed 9 years ago by martin.langhoff

You got it -- sorry I wasn't clear about the version I am targetting...

With the above in mind... does it make sense to review the patchseries? Also, there are some minor ui glitches i mentioned on the ml...

Changed 9 years ago by alsroot

all revert/cherry-picked patches to support original patches

comment:12 Changed 9 years ago by alsroot

new version of sugar-1636.patch w/ rewriting 'uid' metadata field in write()

Changed 9 years ago by alsroot

patch whch is based on sugar-1636-pre.patch

comment:13 Changed 9 years ago by alsroot

could be useful to not rename file if title wasn't changed

comment:14 Changed 9 years ago by martin.langhoff

Reviewed your version of the patch -- complete agreement on all the changes. Looks excellent.

could be useful to not rename file if title wasn't changed

Looking into that.

Changed 9 years ago by martin.langhoff

sugar-1636-take2.patch

comment:15 Changed 9 years ago by martin.langhoff

Uploaded an updated version of the patch. Working on top of Aleksey's patch, it adds

  • Fix to _get_unique_file_name() so that it increments one last item, rather than appending increasing numbers.
  • The write() method differentiates between metadata updates (done inplace), renames (now done in a failsafe manner) and new-file-on-extdist (from DS).
  • Metadata is now written before dealing with the actual data file - this simplifies the flow a bit, and allows the failsafe renames.

All my changes to the patch are in model.py -

comment:16 Changed 9 years ago by tomeu

Just gave it a quick look, two comments:

  • split that big func, then add docstrings (instead of inline comments) if you think it's not clear enough what the code does.
  • don't use abbreviations such as 'md'. It really makes a difference when you have to read lots of code.

comment:17 Changed 9 years ago by martin.langhoff

Tomeu

  • I made several earlier attempts at splitting off part to a function, but you need to pass a lot of vars around. Looking at it together with the "support 0.82 metadata" patch I am working on, the whole model.py probably needs to be refactored a bit, but that clearly needs to be worked on the 'master' branch. Once I am done with these issues I'll post more detailed thoughts and if possible draft patch.
  • Each project with its codestyle, I'll fix that up ;-) I sure read and write a lot of code, long varnames hurt me plenty.

comment:18 Changed 9 years ago by alsroot

Any progress here, should it be picked up?

comment:19 Changed 9 years ago by erikos

  • Bug Status changed from Unconfirmed to New
  • Keywords r! added; r? removed

Can we get a new patch with the changes Tomeu was suggesting?

Changed 9 years ago by erikos

Feedback-when-deleting-files-on-a-USB-Stick http://dev.laptop.org/ticket/10351

Changed 9 years ago by erikos

Refactored patch from Martin and addressed the review comments from Tomeu

comment:20 Changed 9 years ago by erikos

  • Keywords r? added; r! removed

The patch is against 0.84. If people are happy with the patch I will cook one for master.

  • I moved the "Feedback-when-deleting-files-on-a-USB-Stick" to a separate patch.
  • I split the big func
  • Added docstrings and removed inline comments
  • do not decode/encode the preview with base64 as expandentry.py is able to read it directly

comment:21 Changed 9 years ago by martin.langhoff

Reads good to me.

What worries me is: how does the resulting code accomodate a patch to the other Journal issues? Specifically, I am worried about http://dev.laptop.org/ticket/9658

Changed 8 years ago by erikos

Sharing/Backup Journal entries using a storage device (branch master)

Changed 8 years ago by erikos

Convert Journal entries that have been saved to a storage device in 0.82 (branch master)

comment:22 Changed 8 years ago by erikos

  • Milestone changed from 0.84 to 0.92
  • Version changed from 0.84.x to Git as of bugdate

Attached are two patches against the current master implementing the Feature as described in http://wiki.sugarlabs.org/go/Features/Journal_Entry_Sharing

comment:23 Changed 6 years ago by dnarvaez

  • Component changed from journal to sugar

comment:24 Changed 6 years ago by dnarvaez

  • Resolution set to fixed
  • Status changed from new to closed

afaict the patches landed.

comment:25 Changed 6 years ago by dnarvaez

  • Milestone 0.92 deleted

Milestone 0.92 deleted

Note: See TracTickets for help on using tickets.