-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
Debug info: print all workers #1200
base: main-ose
Are you sure you want to change the base?
Conversation
@rfc2822 I've added the info for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a list of all "other" workers which are not mentioned in the account section? Reason is that there may be old workers which have not been cancelled correctly, for instance by migrations.
Maybe we can return the IDs of the workers when we dump the account workers, remember them and at the end print all other workers (= those which were not printed yet). Or something similar.
Also I think it's time to move the debug info generation into a separate class DebugInfoGenerator
, then it won't clutter the model (still UI layer) so much.
cd4315c
to
39680ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AccountsCleanupWorker is not a "sync" worker. I would rename one or both of the dumpSyncWorkersInfo
methods to dumpWorkersInfo
.
Not sure we really need to be able to generate other tables besides "Sync workers" and the new "Generic workers" table. Having a generic generation function seems a bit overengineered to me, but I guess it might come in handy.
@@ -395,6 +397,11 @@ class DebugInfoModel @AssistedInject constructor( | |||
dumpAddressBookAccount(account, accountManager, writer) | |||
} | |||
|
|||
// general-purpose sync workers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are general-purpose sync workers? We have either sync workers or "non-sync" (general purpose) workers.
@@ -395,6 +397,11 @@ class DebugInfoModel @AssistedInject constructor( | |||
dumpAddressBookAccount(account, accountManager, writer) | |||
} | |||
|
|||
// general-purpose sync workers | |||
writer.append("\nSync workers:\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably say "Generic workers" here?
private fun dumpSyncWorkersInfo(): String = | ||
workersInfoTable( | ||
WorkQuery.Builder.fromUniqueWorkNames( | ||
listOf(AccountsCleanupWorker.NAME) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could add PushRegistrationWorker and RefreshCollectionsWorker here too no? Why even have the list in the first place? Would it not be possible to query all existing work and simply subtract the sync workers? Or is that very complicated to do?
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
39680ee
to
e0c1a8b
Compare
Purpose
Right now we only provide the information of the individual sync workers for each account and data type, but no information about the "generic" workers is provided.
Example:
Short description
workersInfoTable
inDebugInfoModel
which allows generating tables for workers in a more generic way.AccountsCleanupWorker
.Checklist