Add a new command to install models #189

matiasdelellis · 2019-11-24T00:27:30Z

Ok, The app store has a limit of 20Mb and our proposed application with the models weighs 27.8 MB.

Well, If we intended to support various models, at some point we had to do something like this.

This is not so happy either, because when you download the files in the application folder we change your checksum, and we will have reports that there are changed files.

Simil that:

I think that at this stage of testing, this is acceptable, but we must place the models in a more appropriate directory.

matiasdelellis · 2019-11-24T00:29:50Z

TODO:

Update docs.
Change CheckrequerimentsTask to warn this ..
Update Makefile to ignore the models on app store...

stalker314314 · 2019-11-24T08:28:30Z

lib/Command/InstallModelsCommand.php

+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class InstallModelsCommand extends Command {


so, I was wondering... can we make this a bit generic. We already discussed this in the form of ./occ face:recommend_memory (here. Can you name this command, someting like ./occ face:setup or ./occ face:prepare or ./occ face:first_time_setup or ./occ face:initialize.... point is, we can use this same command later, not just for downloading models, but this is a place we can tell people "hey, your recommended memory is not high" or "hey, DLib does not exist", or "hey, PDLib cannot be initialized"... instead of having 2-3-5 commands for this, why not make it one? At this moment, it can contain only models, but in the future, we can bundle all various things in here. First next step to merge in this command would be check requirements

OK.. ./occ face:setup
Now without an argument, but then ./occ face:setup model 2, ./occ face:setup memory can be added. I did not like setup, but it is the most generic, and can be consistent with the arguments.

stalker314314 · 2019-11-24T08:32:42Z

lib/Command/InstallModelsCommand.php

+		$this->logger = $output;
+
+		$this->tempFolder = $this->tempManager->getTemporaryFolder('/facerecognition/');
+		$this->modelsFolder = $this->appManager->getAppPath('facerecognition') . '/vendor/models/1/';


so....how this works? If now are our models not part of our install/bundle/zip and they reside inside our app, what happens when users upgrade our app? models do disappear and user need to execute ./occ face:install_models again? If this is not true, ignore my comment. But if this is true, this would be real pain for users. However, I am not sure what other location we can have at our disposal to place models there?

so....how this works? If now are our models not part of our install/bundle/zip and they reside inside our app, what happens when users upgrade our app? models do disappear and user need to execute ./occ face:install_models again?

Wow.. God point..

However, I am not sure what other location we can have at our disposal to place models there?

appdata_INSTANCE_ID seems a good place.. There are saved the preview, avatars, richdocuments templates, deck images, etc. etc.

stalker314314

waiting for my comments

matiasdelellis · 2019-11-24T18:17:20Z

Ok.. The last commit doesn't work and I don't know why ... 😢

Also change the model folder to the appdata._instanceId/facrerecogntion/models/
@stalker314314, If you see anything let me know. I don't know where to look anymore. 😞

The error was:

[matias@nube nextcloud]$ sudo -u apache php occ -vvv face:setup
[sudo] password for matias: 
An unhandled exception has been thrown:
ArgumentCountError: Too few arguments to function OCA\FaceRecognition\Command\BackgroundCommand::__construct(), 0 passed in /var/www/html/nextcloud/lib/private/Console/Application.php on line 222 and exactly 2 expected in /var/www/html/nextcloud/apps/facerecognition/lib/Command/BackgroundCommand.php:51
Stack trace:
#0 /var/www/html/nextcloud/lib/private/Console/Application.php(222): OCA\FaceRecognition\Command\BackgroundCommand->__construct()
#1 /var/www/html/nextcloud/lib/private/Console/Application.php(134): OC\Console\Application->loadCommandsFromInfoXml(Array)
#2 /var/www/html/nextcloud/console.php(96): OC\Console\Application->loadCommands(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#3 /var/www/html/nextcloud/occ(11): require_once('/var/www/html/n...')

stalker314314 · 2019-11-24T18:44:49Z

Did 'doh' commit (8db816c) solved problem, or I should try it locally?:)

matiasdelellis · 2019-11-24T18:48:13Z

Did 'doh' commit (8db816c) solved problem,

No.. 😞

or I should try it locally?:)

Yes please.. At least look at all the diff to see if you notice anything ..

matiasdelellis · 2019-11-24T18:57:34Z

@stalker314314
Now yes.. Although it does not work, with the last commit fix this problem that drove me crazy.!

matiasdelellis · 2019-11-24T19:32:47Z

Now it seems to work. 😄
Please, I need a real test ... and I'm sorry, but the tests again leave them pending to you.. 😞

stalker314314 · 2019-11-24T21:13:25Z

.travis.yml

@@ -54,6 +54,7 @@ before_script:
  - cd apps/$APP_NAME

 script:
+  - ./occ face:setup


I guess it really doesn't matter, but isn't it more logical to have make test and only after face:setup?

I guess it really doesn't matter, but isn't it more logical to have make test and only after face:setup?

I modified it in case there was any test that depended on the models. Change it along with the necessary tests. 😉

lib/Service/ModelService.php

lib/Command/SetupCommand.php

lib/Helper/Requirements.php

stalker314314 · 2019-11-24T21:59:09Z

Everything mostly works, other than stuff I mentioned:) I still cannot visit index.php/settings/admin/facerecognition, so I can try again when that is fixed. But mostly - it does works nicely!

matiasdelellis · 2019-11-24T22:22:41Z

Everything mostly works, other than stuff I mentioned:) I still cannot visit index.php/settings/admin/facerecognition, so I can try again when that is fixed. But mostly - it does works nicely!

All done.
After publishing I want to do some cleaning and code reorganization, but for what we need now I think it's fine ..

matiasdelellis requested a review from stalker314314 November 24, 2019 00:27

stalker314314 reviewed Nov 24, 2019

View reviewed changes

stalker314314 requested changes Nov 24, 2019

View reviewed changes

stalker314314 mentioned this pull request Nov 24, 2019

Release to the app store? #1

Closed

Add occ face:setup command that now hust install the model 1

24a3b7a

matiasdelellis force-pushed the install-models-command branch from d7dd360 to 24a3b7a Compare November 24, 2019 13:46

Add an ModelService that manage installed models

46ff74b

matiasdelellis requested a review from stalker314314 November 24, 2019 18:11

matiasdelellis added 2 commits November 24, 2019 15:35

Adapt ImageProcessingTask to new Requirements

7d0003d

DOH

8db816c

stalker314314 approved these changes Nov 24, 2019

View reviewed changes

DOH. Thanks scrutinizer

c056b7f

matiasdelellis added 3 commits November 24, 2019 16:03

Update function name

1013eaa

Fix typo

9754941

Drop last references of models on makefile, and add setup on travis

0a2a390

matiasdelellis requested a review from stalker314314 November 24, 2019 19:32

stalker314314 reviewed Nov 24, 2019

View reviewed changes

lib/Service/ModelService.php Show resolved Hide resolved

stalker314314 reviewed Nov 24, 2019

View reviewed changes

lib/Command/SetupCommand.php Show resolved Hide resolved

Add missing Exception

8b902ca

stalker314314 reviewed Nov 24, 2019

View reviewed changes

lib/Helper/Requirements.php Show resolved Hide resolved

Dont reinstale models

f0eef22

Move admin pane to modelService

3ba3fed

matiasdelellis requested a review from stalker314314 November 24, 2019 22:22

stalker314314 approved these changes Nov 24, 2019

View reviewed changes

matiasdelellis merged commit b99f6a6 into master Nov 24, 2019

matiasdelellis deleted the install-models-command branch November 25, 2019 00:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new command to install models #189

Add a new command to install models #189

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019 •

edited

stalker314314 Nov 24, 2019 •

edited

matiasdelellis Nov 24, 2019

stalker314314 Nov 24, 2019

matiasdelellis Nov 24, 2019

matiasdelellis Nov 24, 2019

stalker314314 left a comment

matiasdelellis commented Nov 24, 2019

stalker314314 commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

stalker314314 Nov 24, 2019

matiasdelellis Nov 24, 2019

stalker314314 commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

Add a new command to install models #189

Add a new command to install models #189

Conversation

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019 • edited

stalker314314 Nov 24, 2019 • edited

Choose a reason for hiding this comment

matiasdelellis Nov 24, 2019

Choose a reason for hiding this comment

stalker314314 Nov 24, 2019

Choose a reason for hiding this comment

matiasdelellis Nov 24, 2019

Choose a reason for hiding this comment

matiasdelellis Nov 24, 2019

Choose a reason for hiding this comment

stalker314314 left a comment

Choose a reason for hiding this comment

matiasdelellis commented Nov 24, 2019

stalker314314 commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

stalker314314 Nov 24, 2019

Choose a reason for hiding this comment

matiasdelellis Nov 24, 2019

Choose a reason for hiding this comment

stalker314314 commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019

matiasdelellis commented Nov 24, 2019 •

edited

stalker314314 Nov 24, 2019 •

edited