On 04/ 4/11 11:40 AM, Shawn Walker wrote:
On 04/ 4/11 11:28 AM, Brock Pytlik wrote:
On 04/ 4/11 11:25 AM, Shawn Walker wrote:
[snip]
src/modules/server/repository.py:
line 1953: trans_id should never be None, why this change?

Because it can be None, I hit it in testing. If I remember right if you abort/ctrl-c during an open or perhaps an append in JUST the right spot,
then you can get here with trans_id being None. Specifically, if you
ctrl-c after transaction.open has make its directories, but before it
completes, I think you get into this situation. I encountered it while
trying to reproduce bug 17982.

It is an error to call this function without a valid transaction id.
This is not the right fix. You need to find out how trans_id gets set
to None in the first place and fix it there, or allow it to fail as it
was failing before.


Why? It makes more sense for external consumers to have to check exactly
when and why a transaction open failed, then conditionally call abort on
the transaction only under certain conditions? Why shouldn't the
transaction object be able to safely abort itself?

Yes, I believe it does make more sense. 'abandon' is not meant to be used blindly; it meant to be a specific, intentional operation that is used to explicitly abort an existing transaction.

If a caller passes a bogus transaction id, it should fail. That is how the interface was designed to work.

If we want to change this, it should be done as part of a more comprehensive review of the publication process, not as a one-off change. The existing behaviour has served us well enough for a long time now, so I would ask that you not make this change.

Fine. I disagree but see further discussion isn't worth the effort.

As for HOW it gets set, I explained that in the above paragraph. To be
perfectly specific, if a user hits ctrl-c at any point after
Transaction.open or Transaction.append are called and before the
trans_id is set on the object. trans_id can be None. So, on a
TransportTransaction, that's anytime while it's going over transport to
do a publish_open on the repository.

Sounds like an error on the caller's part then. The caller's error handling is wrong.
Fine, I'll write the code you'd like to see then.
Brock

-Shawn

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to