issues with facility import functionality

[indirectlylit]

Observed behavior

The import process has been stuck on 99% for about 10 minutes:

During that time I got a few dozen warnings like this:

WARNING  Validation error for FacilityUser with id 6572fd4d5e34e2efcb482a1737e376f7: {'password': ['This field cannot be blank.']}

A few dozen like this:

WARNING  Validation error for UserSessionLog with id 2aafbc857703fed2e431de94a113b0a7: ['FacilityUser matching query does not exist.']

And about 100,000 entries like this:

WARNING  Validation error for ContentSummaryLog with id 22714685a88c63c4e8c2f12fa7de7343: ['FacilityUser matching query does not exist.']

Then, it hung at 100% for a couple minutes, while strangely saying step "4 of 7":

Waited longer and it did eventually finish:

After this, I tried logging out and then back in using an admin from the newly-imported facility. This seemed to work, but when I went to the Coach page, I saw this:

Expected behavior

1. progress states and step numbers during import better-reflects the time remaining. If necessary we should switch to "indeterminate" progress bar and remove the percentage number.
2. after import, able to access coach reports

User-facing consequences

significant frustration using data syncing

Errors and logs

none apart from those shown above

Steps to reproduce

import kolibri-beta demo facility to a local Kolibri instance

Context

0.14.0 beta 3

[bjester]

I believe we'll need to make the sync an operation that can't be canceled. We've started seeing messages like this when it's been cancelled before.

[bjester]

I'm hoping that getting learningequality/morango#84 merged and released will help resolve this. @jamalex added some improvements to deserialization, but through an issue we were getting the same warnings shown here. Now that we've resolved those with changes in morango, I'm hoping that too will resolve these issues.

[jamalex]

I believe the issue noted in this PR is because you're syncing auto-generated data (from generatedata command or whatnot), which generates user accounts that have empty password fields.
WARNING Validation error for FacilityUser with id 6572fd4d5e34e2efcb482a1737e376f7: {'password': ['This field cannot be blank.']}
We don't allow the password field to be truly blank (and I'm assuming for the password-free login we're using a dummy value, not an empty string), but the generaterealdata command creates users without running validation, and SQLite doesn't really care. But Morango runs validation on all incoming records.

The other models (UserSessionLog and ContentSummaryLog) are not deserializing because they depend on the FacilityUsers that failed to deserialize.

[jamalex]

So I'd make a couple of recommendations here:

• We update generaterealdata to set a dummy password when creating FacilityUsers
• We consider allowing the password field to be blank. This will help with absorbing all this old generated data without it sticking around in undeserializable limbo.
• We allocate more than 1% in the overall import progress to the deserialization stage, as it can in some cases take quite some time.

[bjester]

@jamalex Ah, I didn't notice that! Thanks! I'm still planning on making my changes to some caching stuff related to facilities, which I think is a good improvement overall.

We allocate more than 1% in the overall import progress to the deserialization stage, as it can in some cases take quite some time.

Perhaps I should add more granular progress signals for that? I had thought of that, but didn't end up doing it because I didn't think it would be comparable to network durations.

[jamalex]

I don't think it needs to be internally granular, but maybe allocating... 5% or something to that stage in the overall progression... which I guess just means it would hang at 95% vs 99%. :P

[jamalex]

Actually, looking back to the design spec, not sure on why it would stop at 99% while deserializing:

My understanding from planning that out with @jtamiace was that only the network transfer portions would have progress bars, and the other stages would have a spinner and textual status indicating the stage.

https://www.figma.com/file/LM0YUkbWAOIerZ3EEg0FN6bS/0.14-P2P-Sync?node-id=712%3A0

[bjester]

Ah okay, @jonboiser I'm going to update the UI to make the progress indeterminate for those states unless you have already.

[jonboiser]

Nope, I have not changed it. According to this

kolibri/kolibri/plugins/device/assets/src/views/FacilitiesPage/FacilityTaskPanel.vue

Lines 74 to 77 in 6ae1c0a

	loaderType() {
	return 'determinate';
	},
	statusMsg() {

It's always a 'determinate' loader, so you can change it to 'indeterminate' for the appropriate conditions.

[bjester]

The loader should now be indeterminate when it's in the queue/dequeuing stages. Unfortunately, a large sync can cause those stages to take a while. I've updated the password deserialization in another PR.

[bjester]

This should now be resolved with merge of above PR.

[indirectlylit]

great!