Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the reset_follower management command #13

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bibz
Copy link
Contributor

@bibz bibz commented Nov 22, 2022

Add a new Django management command, reset_follower, that can manually reset the tracking states of a follower (i.e. a ProcessApplication instance). The idea for the follower is to replay the whole history next time it is (manually) synchronised.

Note that this command does not interact with a running system, see the command sync_followers to nudge the system in catching up with the upstream apps.


@johnbywater This is the change I mentioned last week. I did not make any more changes to it but I think it's worth pushing now to open up the discussion (or let me know if you'd rather discuss in an issue thread first).

TODO:

  • Document the command in README
  • Support different databases (Django's using)

Open questions:

  1. edit: no need. We could make the logic a bit more complicated if we locate the system runner to e.g. list all existing process applications, not only the ones with a state stored in the DB. I didn't think it was worth the complexity.

@johnbywater
Copy link
Contributor

Thanks for doing this.

It all looks very neat and tidy, and useful!

The only thought I have, which I'm sure you have thought about already, is that in some cases deleting the tracking records, without also deleting the consequential state of processing the events that have been tracked, might lead to a state in which the events cannot be reprocessed. For example, if a "created" event has been processed by inserting a record, coded under the assumption that there isn't already a record, and attempting to insert such a record twice causes an integrity error.

Regarding your question: it makes good sense to deal with the database records, since if there are no records for a particular application name, then there are no recorded to delete, and locating the system runner in all cases is most likely going to be a tricky business?

Please carry on with this. I like it.

@bibz
Copy link
Contributor Author

bibz commented Nov 22, 2022

The only thought I have, which I'm sure you have thought about already, is that in some cases deleting the tracking records, without also deleting the consequential state of processing the events that have been tracked, might lead to a state in which the events cannot be reprocessed.

This is entirely correct, and that is an important point the documentation should not miss.
Arguably the command output should also state it (and maybe offer a confirmation prompt just in case?)

For example, if a "created" event has been processed by inserting a record, coded under the assumption that there isn't already a record, and attempting to insert such a record twice causes an integrity error.

The first iteration I explored for this command was actually expecting the ProcessApplication to expose either a Django model name to purge directly, or provide a specific hook to be called as the records were cleaned-up.

I do use an added "reset method" in my own examples to prepare the ORM models when the ProcessApplication gets the Created event of a specific aggregate.

I will dig into supporting an additional (optional!*) protocol that offers a reset hook, think something like:

class ResetableFollower(Protocol):
    def on_reset(cls):
        ...

*: I feel quite strongly against name-squatting a method name unless it has been explicitly configured.

Add a new Django management command, `reset_follower`, that can
manually reset the tracking states of a follower (i.e. a
ProcessApplication instance).  The idea for the follower is to
replay the whole history next time it is (manually) synchronised.

Note that this command does not interact with a running system,
see the command `sync_followers` to nudge the system in catching
up with the upstream apps.
@bibz bibz force-pushed the reset-follower-command branch from 3241c06 to 37278a0 Compare November 22, 2022 21:42
@bibz
Copy link
Contributor Author

bibz commented Nov 22, 2022

Updated the command tests for coverage (also output) and support for specifying which upstream app(s) to reset the tracking states of.
I started documenting the command, and clearly need to illustrate better as this deals with advanced concepts.
Still missing the protocol but I like that idea even more now.

@johnbywater
Copy link
Contributor

Hey @bibz! Did you make any progress with this? Hope all is well with you :)

@bibz
Copy link
Contributor Author

bibz commented Mar 20, 2023

Hi @johnbywater! Sorry for the long silence I was busy elsewhere.
As you can imagine, this went off my radar. I'll see if I can resume/wrap up soon or else it's going to be abandoned.

@johnbywater
Copy link
Contributor

No worries @bibz! Thanks for replying. More than happy to have a meeting about it, if you think that would help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants