Support having 2 tasks with the same name on DB

[wil93]

Currently we have three separate uniqueness constraints: (name), (name, contest_id), and (num, contest_id). The first one is not really useful and in some cases it's really annoying (current workaround is adding the contestname_ as a prefix of taskname).

Fixes #765

Before:

Indexes:
    "tasks_pkey" PRIMARY KEY, btree (id)
    "tasks_contest_id_name_key" UNIQUE CONSTRAINT, btree (contest_id, name)
    "tasks_contest_id_num_key" UNIQUE CONSTRAINT, btree (contest_id, num)
    "tasks_name_key" UNIQUE CONSTRAINT, btree (name)
    "ix_tasks_contest_id" btree (contest_id)

After:

Indexes:
    "tasks_pkey" PRIMARY KEY, btree (id)
    "tasks_contest_id_name_key" UNIQUE CONSTRAINT, btree (contest_id, name)
    "tasks_contest_id_num_key" UNIQUE CONSTRAINT, btree (contest_id, num)
    "ix_tasks_contest_id" btree (contest_id)

---

This change is 

[codecov[bot]]

Codecov Report

Merging #998 into master will decrease coverage by 19.9%. The diff coverage is 19.56%.

@@            Coverage Diff             @@
##           master    #998       +/-   ##
==========================================
- Coverage    65.1%   45.2%   -19.91%     
==========================================
  Files         231     231               
  Lines       17746   17763       +17     
==========================================
- Hits        11554    8030     -3524     
- Misses       6192    9733     +3541

Flag	Coverage Δ
#functionaltests	?
#unittests	45.2% <19.56%> (-0.84%)	:arrow_down:

Impacted Files	Coverage Δ
cms/db/task.py	72.91% <ø> (-9.73%)	:arrow_down:
cmscontrib/RemoveTask.py	0% <0%> (ø)	:arrow_up:
cmscontrib/AddTestcases.py	0% <0%> (ø)	:arrow_up:
cmscontrib/AddSubmission.py	76.41% <100%> (ø)	:arrow_up:
cmscontrib/ImportTask.py	29.76% <11.76%> (-51.64%)	:arrow_down:
cmscontrib/AddStatement.py	32.2% <21.42%> (-43.66%)	:arrow_down:
cmscontrib/ImportDataset.py	37.31% <25%> (-45.77%)	:arrow_down:
cmscontrib/ImportContest.py	77.5% <55.55%> (-3.27%)	:arrow_down:
cmscontrib/importing.py	39.44% <8.33%> (-38.77%)	:arrow_down:
cmsranking/__init__.py	0% <0%> (-100%)	:arrow_down:
... and 105 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d75b457...7f5590b. Read the comment docs.

[lw]

Are you serious? Did you really not even try to fix all the things that would break because of this?

Here are just the first ones that came to my mind: AddTestcases, AddStatement, ImportTask, RemoveTask, RWS, ...

[lw]

I searched a bit more and also ImportContest and ImportDataset would break.

[wil93]

I assume by "will break" you mean "will print a SQL integrity error instead of a user-friendly error message, in some cases" (especially given the fact that all workflows used until now, including the test suite, are working with no problems).

Anyway, OK, I will look into adding more user-friendly error messages. It must be said though that there are some situations where you can get this kind of SQL error printouts already (if you edit tasks in the wrong way, using AWS ...) so I would tend to assume that "contest admins" should mostly be fine with such errors.

[lw]

No, by "break" I mean it will become impossible to add a statement to a task if there is another one with the same name in another contest, or that the wrong task will be removed, or that a dataset will be added to the wrong task, or that data will be overwritten. In short, tools will not do what they're supposed to. Like, they will break.

On Thu, Aug 23, 2018, 19:00 William Di Luigi [email protected] wrote:

I assume by "will break" you mean "will print a SQL integrity error instead of a user-friendly error message, in some cases" (especially given the fact that all workflows used until now, including the test suite, are working with no problems).

Anyway, OK, I will look into adding more user-friendly error messages. It must be said though that there are some situations where you can get this kind of SQL error printouts already (if you edit tasks in the wrong way, using AWS ...) so I would tend to assume that "contest admins" should mostly be fine with such errors.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cms-dev/cms/pull/998#issuecomment-415361452, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHX6oG40u0cpyVt0mxK384_5pwV7zzMks5uTn01gaJpZM4WICRa .

[lw]

The list I gave you earlier is just all the tools that issue SELECT queries filtering by task name without also filtering by contest. All these queries will stop working the way they're expected and need to be fixed. Since there's small differences in behavior whether a query is fetched with first(), one() or scalar() (and that now I'm not at my computer) I can't really tell you which tool breaks in which way.

And then there's RWS, which won't be fun to fix.

On Thu, Aug 23, 2018, 19:16 Luca Wehrstedt [email protected] wrote:

No, by "break" I mean it will become impossible to add a statement to a task if there is another one with the same name in another contest, or that the wrong task will be removed, or that a dataset will be added to the wrong task, or that data will be overwritten. In short, tools will not do what they're supposed to. Like, they will break.

On Thu, Aug 23, 2018, 19:00 William Di Luigi [email protected] wrote:

I assume by "will break" you mean "will print a SQL integrity error instead of a user-friendly error message, in some cases" (especially given the fact that all workflows used until now, including the test suite, are working with no problems).

Anyway, OK, I will look into adding more user-friendly error messages. It must be said though that there are some situations where you can get this kind of SQL error printouts already (if you edit tasks in the wrong way, using AWS ...) so I would tend to assume that "contest admins" should mostly be fine with such errors.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cms-dev/cms/pull/998#issuecomment-415361452, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHX6oG40u0cpyVt0mxK384_5pwV7zzMks5uTn01gaJpZM4WICRa .

[wil93]

Oh, now I see, good points.

I guess I'll replace most of "task name" requests with "task id"...would that work? These are admin tools anyway so we can assume an admin knows how to find out the ID of a task. We could eventually add some scripts to ease data browsing from the command line (cmsList (tasks|users|...)) so that to retrieve the IDs one doesn't need to use AWS or psql.

[stefano-maggiolo]

I would prefer a disambiguation (at least via command line argument, possibly interactive too) to avoid breaking existing scripts.

[wil93]

OK, I will add an optional -t <task_id> argument that overrides the positional task_name when specified, and becomes mandatory when the task name is ambiguous.

[stefano-maggiolo]

[Note this might not cover all issues, in some other places you might need to do something different...]

[lw]

I would prefer a --contest-name/-c option. We already have it, for example in AddTestcases, except that now it's only used to verify that the task with the given name belongs to that contest. So it would be nice to fix that behavior, and to extend that flag to all affected commands.

[lw]

As for RWS, it may be enough to just fix the PS side. PS should keep the short_name field of the task set to its name, but the identifier (the last component of the URL) should now become the combination of the contest name and the task name (with some precautions to avoid ambiguities of course).

[lw]

Also, in AWS there are some views where we list all tasks, for all contests, by name. We should now add their contests' names as well, to allow the admins to select without ambiguity.

[wil93]

I think the id of the task must be specified in some cases. There can be 10 tasks with the same name and not tied to any contest

[wil93]

OK, now almost everything works. Soon I will fix the rest.

(cms) wil93@xps13:~/git/cms$ cmsRemoveTask batch
2018-08-24 01:03:16,075 - INFO [<unknown>] Using configuration file /usr/local/etc/cms.conf.
2018-08-24 01:03:16,396 - ERROR [<unknown>] Task name is ambiguous, please use the -t argument to specify a valid ID:
 ID =   1 | batch (Batch) | con_test
 ID =  10 | batch (Batch) | <none>
 ID =  11 | batch (Batch) | <none>
2018-08-24 01:03:16,396 - INFO [<unknown>] Error while importing, no changes were made.

And this is when I specify an ID:

(cms) wil93@xps13:~/git/cms$ cmsRemoveTask batch -t 10
2018-08-24 01:04:25,519 - INFO [<unknown>] Using configuration file /usr/local/etc/cms.conf.
This will delete task `batch' and all related data, including submissions. Are you sure? [y/N] y
2018-08-24 01:04:28,023 - INFO [<unknown>] Task `batch' removed.

[stefano-maggiolo]

Ready to review again?

[wil93]

Well it's not finished yet but the tools should work. AWS needs some aesthetic work but it's not broken. I didn't look into RWS yet.