Skip to content

Move logic into private methods that take in a connection #40

Description

@dwhswenson

This has no effect on current functionality, but improves ability to make custom subclasses.

Is your feature request related to a problem? Please describe.

I have ways I'm interested in extending the functionality of a TaskStatusDB in a subclass, but it needs to be part of the same transaction. Currently, there's no way to extend what happens in the transaction.

Describe the solution you'd like

For example, we'd have:

def _check_out_task(self, conn):
    ... # does the logic

def check_out_task(self):
    _logger.info("Checking out a new task")
    with self.engine.begin() as conn:
        result = self._check_out_task(conn)

    # code to check result

Compare with the current implementation:

exorcist/exorcist/taskdb.py

Lines 460 to 480 in 95cbc13

def check_out_task(self):
# TODO: may need move this to a single attempt function and wrap it
# in while loop to catch NoStatusChange errors until we have a
# successful checkout
_logger.info("Checking out a new task")
subq = (
sqla.select(self.tasks_table.c.taskid)
.where(self.tasks_table.c.status == TaskStatus.AVAILABLE.value)
# .order_by(tasks_table.c.priority.desc()) # FUTURE: priority
.limit(1)
.scalar_subquery()
)
with self.engine.begin() as conn:
update_stmt = self._task_row_update_statement(
taskid=subq,
status=TaskStatus.IN_PROGRESS,
is_checkout=True,
old_status=TaskStatus.AVAILABLE,
).returning(self.tasks_table.c.taskid)
result = list(conn.execute(update_stmt))

We move the logic that happens inside the transaction to a separate function. This allows my subclass to implement additional logic in that transaction:

# in MyTaskStatusDB
def _check_out_task(self, conn):
    self._do_my_extra_stuff(conn)
    return super()._check_out_task(conn)

Describe alternatives you've considered

Require duplication of code in subclasses: I can copy-paste the current implementation, and then add code to meet my needs. But this kind of ruins the point of inheritance.

Additional context

The downsides, as I see are:

  1. There may be some tiny performance hits because we'd move a few SQL statement definitions inside the connection, instead of keeping them outside.
  2. It does technically add an extra method.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions