Page MenuHomeSoftware Heritage

Make create_task_type idempotent
ClosedPublic

Authored by anlambert on Feb 17 2020, 4:56 PM.

Details

Summary

There is no reason to raise an error when a task type has already been created and it enables to stop leaking
psycopg2 IntegrityError exception as part of the scheduler interface.

Diff Detail

Repository
rDSCH Scheduling utilities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

olasd requested changes to this revision.Feb 18 2020, 2:54 PM
olasd added a subscriber: olasd.

Instead of doing this, I think we should really make create_task_type idempotent. Looks like no users of this API actually depend on the "fail if type already exists" behavior.

If we do that, we'll stop leaking psycopg2 IntegrityErrors as part of the scheduler interface, which is, er, gross.

This revision now requires changes to proceed.Feb 18 2020, 2:54 PM
In D2683#64235, @olasd wrote:

Instead of doing this, I think we should really make create_task_type idempotent. Looks like no users of this API actually depend on the "fail if type already exists" behavior.

If we do that, we'll stop leaking psycopg2 IntegrityErrors as part of the scheduler interface, which is, er, gross.

Ack, seems better indeed.

Update: make create_task_type idempotent instead of raising an exception when a task type has already been created.

swh/scheduler/backend.py
102

Wouldn't it make more to do update?

swh/scheduler/backend.py
102

Indeed, it could be used to change task type setup. But then it is no more a creation operation. @olasd, your opinion on this ?

anlambert retitled this revision from test_scheduler: Fix a RPC test after recent changes in swh-core to Make create_task_type idempotent.Feb 18 2020, 4:59 PM
anlambert edited the summary of this revision. (Show Details)
olasd added inline comments.
swh/scheduler/backend.py
102

I'd rather have policy updates be a manual operation, so ack for on conflict do nothing.

This revision is now accepted and ready to land.Feb 19 2020, 2:09 PM

But then, errors would pass silently...

If the issue is only leaking the postgresql exception, we could catch it and reraise another one.