Bug 45165 - [RESTful Import API] start import from further within import framework
[RESTful Import API] start import from further within import framework
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: HTTP-API (Kelvin)
UCS@school 4.2
Other Linux
: P5 normal (vote)
: UCS@school 4.3 v6
Assigned To: Daniel Tröder
Ole Schwiegert
:
: 47690 (view as bug list)
Depends on: 47975 45673
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-08 17:02 CEST by Daniel Tröder
Modified: 2018-11-16 11:48 CET (History)
2 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Security
Max CVSS v3 score:


Attachments
text out of bounds (41.90 KB, image/png)
2018-10-04 09:38 CEST, Ole Schwiegert
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2017-08-08 17:02:10 CEST
The HttpApiImportFrontend(UserImportCommandLine) to start the import job should be replaced by something that receives meaningful exceptions. Those would end up in the tasks result storage and be helpful to find problems.
Comment 1 Daniel Tröder univentionstaff 2018-04-17 13:36:48 CEST
Work on this may be a basis for Bug #46712.
Comment 2 Daniel Tröder univentionstaff 2018-09-20 15:29:24 CEST
* The startup of an import job has been split into preparation (setup logging, configuration, config checks etc) and the actual run. This allows for finer grained error control/messages.
* The list of error messages and the textual summary is now retained when when exceptions happen. This way the textual summary can (as a "result") be passed to frmo the celery process to the frontend (as the result is stored in the DB).
* Error messages and tracebacks are now logged immediately and not after the import. That kind of fixes the chronological order in the logfile and removes the ugly tracebacks at the end.
* The list of error messages now has a better layout.
* As the frontend class calls prepare_import() and do_import() directly, it is not effected anymore by main()s return -> sys.exit() calls.

As a result that dry-run always displays a "success" or "error" message and the textual summary now.


Commit are in branch dtroeder/45165_start_http_api:

291ad7cad Bug #45165: move job states into separate module to avoid Django initialization when they are required
508089987 Bug #45165: improve format of error list in textual summary
5e4eef80c Bug #45165: collect errors and summary even when an exception happens
16ea1c56e Bug #45165: split preparing and running the import
28fa4e3b4 Bug #45165: add error to correct list (enhancement over b997e83b2 from Bug #45673)
08662bcae Bug #45165: errors and tracebacks are now logged where they happen
dbee4a81d Bug #45165: use splitted preperation and running of import -> no more process exit, catch all excpetions and adapt state and message accordingly
6aaf6b55f Bug #45165: adapt to changed celery job result strategy
Comment 3 Daniel Tröder univentionstaff 2018-10-02 12:54:02 CEST
*** Bug 47690 has been marked as a duplicate of this bug. ***
Comment 4 Ole Schwiegert univentionstaff 2018-10-04 09:38:34 CEST
Created attachment 9690 [details]
text out of bounds

The error text can get out of bounds in the UMC module. That does not look very good.
Comment 5 Daniel Tröder univentionstaff 2018-10-04 12:06:21 CEST
(In reply to Ole Schwiegert from comment #4)
> The error text can get out of bounds in the UMC module. That does not look
> very good.
The import cannot know the width of the users browser window.
IMHO the frontend should either
* use line breaks in a text window or
* or add line breaks in JS or
* send the maximum line length to the UMC module, so it can reformat the text.
Comment 6 Daniel Tröder univentionstaff 2018-10-15 12:35:54 CEST
(In reply to Daniel Tröder from comment #5)
> (In reply to Ole Schwiegert from comment #4)
> > The error text can get out of bounds in the UMC module. That does not look
> > very good.
> The import cannot know the width of the users browser window.
> IMHO the frontend should either
> * use line breaks in a text window or
> * or add line breaks in JS or
> * send the maximum line length to the UMC module, so it can reformat the
> text.
As discussed, this aspect will be handled separately in Bug #47975.
Comment 7 Ole Schwiegert univentionstaff 2018-10-16 11:08:48 CEST
Besides the mentioned GUI problem before, the code works as expected. The log now contains errors and messages in the order they are raised/added and a summary of the import is printed out.

After building I can do the final testing and Verify the bug.

Though I would like to consider this bug being blocked by #47975 instead of the other way around since I would dislike publishing this feature request without first solving the displaying problem.
Comment 8 Daniel Tröder univentionstaff 2018-10-16 12:38:50 CEST
(In reply to Ole Schwiegert from comment #7)
> Though I would like to consider this bug being blocked by #47975 instead of
> the other way around since I would dislike publishing this feature request
> without first solving the displaying problem.
Oh - that as a misunderstanding.
I switched depends and blocks.
Commits were merged to 4.3, packages built and advisories added:

ucs-school-import 16.0.2-59
ucs-school-umc-import 1.0.1-3
Comment 9 Ole Schwiegert univentionstaff 2018-10-17 10:22:29 CEST
Changelog & Advisories: OK

Changes work, same as in comment #7
Comment 10 Sönke Schwardt-Krummrich univentionstaff 2018-11-16 11:48:14 CET
UCS@school 4.3 v6 has been released.

https://docs.software-univention.de/changelog-ucsschool-4.3v6-de.html

If this error occurs again, please clone this bug.