485731 - [silme] 'path' in FileClient.write_object should support both directories and files

Status: RESOLVED FIXED

(Mozilla Localizations :: Infrastructure, defect)

Description

[Staś Małolepszy :stas]

Attachment: Patch

I think that all across Silme, when working with l10nobjects, 'path' is the full path to the file, except for the FileClient.write_object class method. This method requires that 'path' point to a directory, and than appends the filename via object.id. So if you have a variable with the path to a file, you have to os.path.dirname it. Not a major pain, but would be nice to unify this.

Proposed patch is attached.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm not convinced, but I'll investigate it more tomorrow.

It definitely will have to be patched against the tree after bug 479155 gets solved.

Comment 2

[Adrian Kalla [:adriank]]

I agree with Stas here: it's better to have it here the same way like everywhere else.

Comment 3

[Adrian Kalla [:adriank]]

Stas: 
1) The patch needs to be updated... ;) 
(the code for choosing the name is here:
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l98
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l84
http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l71
)

2) Something I was thinking about:
os.path.isdir(path) will be always True when writing a whole L10nPackage, because we create this path-directories here: http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/file/4aa68586ff68/lib/silme/io/file.py#l117
So in case of writing a L10nPackage, we will use always a object.id as file name.
The question is: is there an other (working) way a developer can submit names for objects to be written...?
I think not, but I'm asking just to not be surprised when it's too late ;)

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

the question is if structure.id should be related to its file name.

I believe it should, but maybe there are some compelling reasons to split it in the future.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

http://hg.mozilla.org/users/zbraniecki_mozilla.com/silme/rev/81ad67f20b0c