Didn't make batch affecting globally yet.
I guess we could move batch into koji.conf and ClientSession options next like
https://pagure.io/fork/julian8628/koji/commits/global-batch

I don't like global approach. There is many cases, where it makes sense to uses batches, on the other hand a lot of other usecases should be done in one transaction or not at all. Enabling batch for all multicall calls seems to me too dangerous. So, I would rather merge this request and use batch explicitly, where we are sure that it is safe to.

I don't like global approach. There is many cases, where it makes sense to uses batches, on the other hand a lot of other usecases should be done in one transaction or not at all. Enabling batch for all multicall calls seems to me too dangerous. So, I would rather merge this request and use batch explicitly, where we are sure that it is safe to.

Yes, agree.

Maybe batch=None? It seems to me like it is more evident that batch mode will not be used, but maybe it is only personal taste.

As we are already doing this for large number of calls, generator could save us some memory. (switch from [ to () It should work even on py2.5

updated

I would add more explicit warning as users needn't to notice danger of separate transactions.

@tkopecek
I added one warning msg in clone-tag and updated docstr for multiCall()

:thumbsup:

I don't think we need the warnings here. They're going to scare users needlessly. This command has never been a single transaction (different aspects are changed in different multicalls, or in individual calls), so there has always been a risk that an interrupted run would leave things in a broken state or that other users might see see partial work.

That multiCall() docstring has gotten unwieldy. I've rewritten it here:

https://github.com/mikem23/koji-playground/commits/pagure/pr/900

Do we want to default to batch=1000? How common is #885?

That multiCall() docstring has gotten unwieldy. I've rewritten it here:
https://github.com/mikem23/koji-playground/commits/pagure/pr/900

+1, and rebased those commits

Do we want to default to batch=1000? How common is #885?

Do you mean to using batch=1000 for multiCall globally?
It looks we have some other calls which might be too long as well, like davidlt tense, and CGImport, but they are not related to multiCall
in CLI, multiCall is used in

• anon_handle_list_channels
• anon_handle_list_hosts
• handle_set_pkg_arches
• handle_set_pkg_owner
• handle_unblock_pkg
• handle_edit_host
• handle_add_pkg
• handle_block_pkg
• handle_remove_pkg
• handle_disable_host
• handle_enable_host
• handle_prune_signed_copies

But, they are barely able to reach the size limit even 1000.
and it's also invoked in koji-gc, koji-shadow, kojira, kojiweb.
it looks the call in kojira could be very big by chance.

Do you mean to using batch=1000 for multiCall globally?

I mean in this particular command. Currently the --batch option has a default value of 1000.

It's not clear to me whether we want that default to stay as is (work around #885 by default at the cost of extra transactions), or leave batching off by default (the original behavior). That's why I'm wondering how frequent #885 is.

It looks we have some other calls which might be too long as well, like davidlt tense, and CGImport, but they are not related to multiCall

Clients can avoid this in CGImport by uploading the metadata as a file rather than passing it in as an arg.

I'm ok with this command defaulting to batch=1000, I just want to be sure we're thinking about the reasons and risks.

If we're all happy with the current changes, I should be able to merge them on Monday.

1000 can cover the requirement of RCM without adding --batch param on this command, and won't break other common using.
I would +1 on default 1000 here.

:thumbsup:

Commit 967704b9 fixes this pull-request