Implement project admin commands. Fixes #8 #26

tvirolai · 2017-10-09T16:21:17Z

A basic implementation for project administration commands. The annif init command also now checks for orphaned indexes and removes them if necessary.

osma

Code looks good. However, I'm not sure that the commands use the optimal output format. Especially the show-project output looks messy.

How about using a multiple column format similar to the virsh tool that is used for managing KVM virtual machines? See https://www.centos.org/docs/5/html/5.2/Virtualization/sect-Virtualization-Commands_for_Red_Hat_Virtualization-virsh_the_command_line_interface_tool_for_virtualization.html for some examples.

The show-project command could use a key-value output format similar to the virsh dominfo command.

tvirolai · 2017-10-10T10:08:44Z

Very true. I'll look into this.

tvirolai · 2017-10-10T11:04:13Z

Now show-project produces output formatted like this:

$ annif show-project herttilei
details about project 'herttilei':

project name:        herttilei
project language:  en
project analyzer:   english
index:                    annif
score:                    0.2876821

list-projects works like this:

(annif_env) ➜  annif git:(issue8) annif list-projects                                               
Currently available projects:

1. herttilei (language: en, analyzer: english)
2. jaahas (language: armenian, analyzer: armenia)

What do you think?

osma · 2017-10-10T11:05:16Z

Much better now. In show-project output, score is completely useless and index is probably redundant too.

tvirolai · 2017-10-10T11:06:09Z

Ok, I'll remove 'em.

osma · 2017-10-10T11:08:40Z

How about this mocked up output style:

$ annif list-projects
Project ID     Language   Analyzer
----------------------------------
herttilei      en         english
jaahas         armenian   armenia

$ annif show-project herttilei
Project ID: herttilei
Language:   en
Analyzer:   english

tvirolai · 2017-10-10T11:09:24Z

I also tweaked the test so that running them doesn't affect any other indices that may be in the database.

tvirolai · 2017-10-10T11:10:22Z

Ok, looks nice. I'll update.

osma · 2017-10-10T11:31:31Z

list-projects now fails for me with this error:

  File "/home/ozone/git/Annif/annif/annif.py", line 87, in listprojects
    parsed += parseRow(p['name'], p['language'], p['analyzer'])
  File "/home/ozone/git/Annif/annif/annif.py", line 81, in parseRow
    return "{0: <15}{1: <15}{2: <15}\n".format(col1, col2, col3)
TypeError: non-empty format string passed to object.__format__

tvirolai · 2017-10-10T12:33:37Z

Weird. The only way I managed to reproduce this error was to first create a project with insufficient arguments (e.g. provide only a name but not language or analyzer information) and then try to list the available projects.

Creating projects with insufficient arguments is no longer possible but it seems that the code analyzers now deem the createProject function too complex. I'll refactor.

osma · 2017-10-10T12:36:02Z

That's probably the reason. I had created the project without specifying all the parameters.

osma · 2017-10-10T12:37:04Z

annif/annif.py

- # Create an index for the project
- index.create(index=proj_indexname)
+ if not projectid or not language or not analyzer:
+ print('Usage: annif create-project <projectId> --language <lang>'


Missing a space after <lang>

osma · 2017-10-10T13:22:03Z

annif/annif.py

 projects = [x['_source'] for x in results['hits']['hits']]
+
+ def parseRow(col1, col2, col3):


Nitpick on naming: I would call this function formatRow, not parseRow. What it does is the opposite of parsing...

osma · 2017-10-10T14:22:40Z

annif/annif.py

@@ -32,10 +33,15 @@
 }


-def parseIndexname(projectid):
+def parse_indexname(projectid):


shouldn't this be format_indexname or even better format_index_name?

Yep, fixing.

osma · 2017-10-10T14:23:21Z

tests/test_annif.py

+ assert not index.exists(annif.parse_indexname(FAILED_PROJECT))
+
+
+def test_showproject():


rename to test_show_project?

osma · 2017-10-10T14:23:40Z

tests/test_annif.py



-def test_dropProject():
- result = runner.invoke(annif.dropProject, [TEMP_PROJECT])
+def test_dropproject():


ditto, test_drop_project

Fixed in the latest commit.

osma · 2017-10-10T14:24:51Z

tests/test_annif.py

@@ -25,31 +30,51 @@ def test_start():

 def test_init():
 result = runner.invoke(annif.init)
- assert index.exists('annif')
+ assert index.exists(annif.annif.config['INDEX_NAME'])
 assert result.exit_code == 0


 def test_listprojects():


rename to test_list_projects?

Fixed in the latest commit.

Implement project admin commands. Fixes #8

375431d

tvirolai requested a review from osma October 9, 2017 16:21

osma reviewed Oct 10, 2017

View reviewed changes

Cleaner output in list-projects and show-project

8028e5d

Remove redundant data from show-project output

608aec9

Tweak output parsing

15f3933

Tweaking create-project

12d5f40

osma reviewed Oct 10, 2017

View reviewed changes

tvirolai added 3 commits October 10, 2017 15:43

Refactor createProject

dc6b8fc

Refactor createProject some more

7fd47d4

Tweak tests

e565b58

osma reviewed Oct 10, 2017

View reviewed changes

tvirolai added 5 commits October 10, 2017 16:22

Refactor too complex functions

3720219

Rename parseRow -> formatRow

a2e79a0

Simplify code further

ef65c45

Simplify code further

5b41b9f

Refactor function names to snake_case

6a13479

osma reviewed Oct 10, 2017

View reviewed changes

tvirolai added 6 commits October 11, 2017 10:05

Fix code style issues reported by Scrutinizer

f2b79a3

Rename rest of the functions

b2e91e7

Try to fix remaining code analyzer issues

6027677

Try to fix remaining code analyzer issues

36bf277

Try to fix remaining code analyzer issues

14d75a0

Try to fix remaining code analyzer issues

51732db

tvirolai merged commit 29db2c6 into master Oct 11, 2017

tvirolai deleted the issue8 branch October 11, 2017 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement project admin commands. Fixes #8 #26

Implement project admin commands. Fixes #8 #26

tvirolai commented Oct 9, 2017

osma left a comment

tvirolai commented Oct 10, 2017

tvirolai commented Oct 10, 2017 •

edited

Loading

osma commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017 •

edited

Loading

tvirolai commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017

osma Oct 10, 2017

osma Oct 10, 2017

osma Oct 10, 2017

tvirolai Oct 11, 2017

osma Oct 10, 2017

tvirolai Oct 11, 2017

osma Oct 10, 2017 •

edited

Loading

tvirolai Oct 11, 2017

osma Oct 10, 2017

tvirolai Oct 11, 2017

		projects = [x['_source'] for x in results['hits']['hits']]

		def parseRow(col1, col2, col3):

		assert not index.exists(annif.parse_indexname(FAILED_PROJECT))


		def test_showproject():

Implement project admin commands. Fixes #8 #26

Implement project admin commands. Fixes #8 #26

Conversation

tvirolai commented Oct 9, 2017

osma left a comment

Choose a reason for hiding this comment

tvirolai commented Oct 10, 2017

tvirolai commented Oct 10, 2017 • edited Loading

osma commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017 • edited Loading

tvirolai commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017

tvirolai commented Oct 10, 2017

osma commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osma Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvirolai commented Oct 10, 2017 •

edited

Loading

osma commented Oct 10, 2017 •

edited

Loading

osma Oct 10, 2017 •

edited

Loading