On 2020/12/23 14:09, Nathan Hartman wrote:
> On Tue, Dec 22, 2020 at 10:13 AM Yasuhito FUTATSUKI
> <futatuki_at_yf.bsdclub.org> wrote:
>>
>> On 2020/08/29 2:49, C. Michael Pilato wrote:
>>>>> Committed revision 1880967.
>>>>>
>>>>> I made only minor comment/formatting changes.
>>>>
>>>> Shouldn't the change be documented somewhere for users of the
>>>> bindings? (E.g., release notes, API errata, API documentation…)
>>>>
>>>
>>> Yes, I think that makes sense. The release notes for sure. As for API
>>> docs/errata ... do we even have such a thing for the bindings?
>>
>> I'm very sorry for my late reply.
>>
>> This change affects all programs using return value of svn.fs.commit_txn.
>> So I think api-errata is needed.
>>
>> As you all and I know my English is too bad, however I wrote it as a
>> starting point. Please refine or rewrite if the errata is needed.
>
> Thank you for remembering to do this.
>
> I rewrote it differently, because I was not familiar with the context
> of r1880967, so I think this was helpful as an exercise for me to
> learn more about how the C APIs and Python bindings interact.
>
> Please review and tell me what you think... (Maybe some of the facts
> are wrong!)
>
> [[[
>
> svn.fs.commit_txn is a Python wrapper for the C API svn_fs_commit_txn.
>
> Like other C APIs, svn_fs_commit_txn uses its return value to indicate
> success or error. In addition, it returns two more values by pointer
> arguments: 'conflict' and 'new_rev'. These are mutually exclusive:
> either the commit succeeds, indicated by a valid revision number in
> 'new_rev' or the commit fails, with details in 'conflict'. The API's
> return value, taken together with these two additional pieces of
> information, tell the full story of the function's success or failure.
>
> In particular, if a commit succeeds but an error occurs in post-commit
> processing, the API will return an error but still indicate the
> successful commit by returning a valid revision number in 'new_rev'.
>
> When the C API returns successfully, the Python wrapper returns the
> two values 'conflict' and 'new_rev' in a 2-tuple. If the C API returns
> an error, the Python wrapper raises an exception.
>
> Unfortunately, this suffers the following problems:
>
> (1) An exception prevents the wrapper from returning the 2-tuple, so
> the caller cannot know whether a successful commit occurred or not.
>
> (2) When there is no exception, 'conflict' is always None, defeating
> the purpose of trying to return it in a 2-tuple.
>
> (3) This is inconsistent with svn.repos.commit_txn, which only returns
> a 'new_rev' singleton on success.
>
> For the 1.15 release, svn.fs.commit_txn is made consistent with
> svn.repos.commit_txn in returning a 'new_rev' singleton on success.
>
> In addition, we introduce a framework to add attributes to
> 'SubversionException' to return additional values to the caller in
> situations such as above. (This new feature is mentioned for
> completeness but is not part of the errata.)
>
> API users who relied on the previous 2-tuple return value should
> adjust their code to process the 'new_rev' singleton on successful
> return.
>
> ]]]
>
> Thoughts?
The last paragraph is for "== Impact on API Users ==" section, I
think. (Yes, my draft also missed this mark of the section.)
I think the error in this erratum is (3) only, and the fix might
be a change type of return value of svn.repos.commit_txn. This was
caused by r845217, which fixed the return type of svn.fs.commit_txn
to 2-tuple and wasn't applied to svn.repos.commit_txn. So the
explanation of these API is only for justification of the fix.
Also as the target of this api-errata are users of those API,
I think that so detailed explanation of APIs itself does not need.
However, as a document for these Python wrapper APIs, this
explanation is very useful.
Cheers,
--
Yasuhito FUTATSUKI <futatuki_at_yf.bsclub.org>
Received on 2020-12-23 19:05:57 CET