Bug 1058549 - Key value import gives 500 error for undefined systems
Summary: Key value import gives 500 error for undefined systems
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: web UI
Version: develop
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: 0.15.4
Assignee: Amit Saha
QA Contact: tools-bugs
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-01-28 02:45 UTC by xjia
Modified: 2018-02-06 00:41 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-18 01:38:44 UTC
Embargoed:


Attachments (Terms of Use)

Description xjia 2014-01-28 02:45:26 UTC
Description of problem:
Key value import gives 500 error for undefined systems

Version-Release number of selected component (if applicable):
develop

How reproducible:
easy to reproduce

Steps to Reproduce:
1. Setup a new beaker server, and add one lab-controller to this server. Now there's no machine.
2. Import CSV file which contains key/value.


Actual results:
It shows ISE500.
Jan 26 21:01:11 beaker beaker-server[22402]: cherrypy.msg INFO HTTP: Page handler: <bound method CSV.action_import of <bkr.server.CSV_import_export.CSV object at 0x7fee6f2c4e50>>
Jan 26 21:01:11 beaker beaker-server[22402]:  Traceback (most recent call last):
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/CherryPy-2.3.0-py2.6.egg/cherrypy/_cphttptools.py", line 121, in _run
Jan 26 21:01:11 beaker beaker-server[22402]:      self.main()
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/CherryPy-2.3.0-py2.6.egg/cherrypy/_cphttptools.py", line 264, in main
Jan 26 21:01:11 beaker beaker-server[22402]:      body = page_handler(*virtual_path, **self.params)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "<string>", line 3, in action_import
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/turbogears/controllers.py", line 361, in expose
Jan 26 21:01:11 beaker beaker-server[22402]:      *args, **kw)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/bkr/server/wsgi.py", line 54, in run_with_transaction_noop
Jan 26 21:01:11 beaker beaker-server[22402]:      return func(*args, **kwargs)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/turbogears/controllers.py", line 244, in _expose
Jan 26 21:01:11 beaker beaker-server[22402]:      @abstract()
Jan 26 21:01:11 beaker beaker-server[22402]:    File "<generated code>", line 0, in _expose
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/peak/rules/core.py", line 153, in __call__
Jan 26 21:01:11 beaker beaker-server[22402]:      return self.body(*args, **kw)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/turbogears/controllers.py", line 390, in <lambda>
Jan 26 21:01:11 beaker beaker-server[22402]:      fragment, options, args, kw)))
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/turbogears/controllers.py", line 425, in _execute_func
Jan 26 21:01:11 beaker beaker-server[22402]:      output = errorhandling.try_call(func, *args, **kw)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/turbogears/errorhandling.py", line 77, in try_call
Jan 26 21:01:11 beaker beaker-server[22402]:      return func(self, *args, **kw)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "<string>", line 2, in action_import
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/bkr/server/identity.py", line 242, in require
Jan 26 21:01:11 beaker beaker-server[22402]:      return func(*args, **kwargs)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/bkr/server/CSV_import_export.py", line 161, in action_import
Jan 26 21:01:11 beaker beaker-server[22402]:      self.from_csv(system, data, log)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/bkr/server/CSV_import_export.py", line 230, in from_csv
Jan 26 21:01:11 beaker beaker-server[22402]:      csv_types[csv_type]._from_csv(system, data, csv_type, log)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib/python2.6/site-packages/bkr/server/CSV_import_export.py", line 814, in _from_csv
Jan 26 21:01:11 beaker beaker-server[22402]:      session.flush([key_value])
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/scoping.py", line 139, in do
Jan 26 21:01:11 beaker beaker-server[22402]:      return getattr(self.registry(), name)(*args, **kwargs)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/session.py", line 1400, in flush
Jan 26 21:01:11 beaker beaker-server[22402]:      self._flush(objects)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/session.py", line 1481, in _flush
Jan 26 21:01:11 beaker beaker-server[22402]:      flush_context.execute()
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/unitofwork.py", line 302, in execute
Jan 26 21:01:11 beaker beaker-server[22402]:      rec.execute(self)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/unitofwork.py", line 446, in execute
Jan 26 21:01:11 beaker beaker-server[22402]:      uow
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/orm/mapper.py", line 1887, in _save_obj
Jan 26 21:01:11 beaker beaker-server[22402]:      execute(statement, params)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/base.py", line 1191, in execute
Jan 26 21:01:11 beaker beaker-server[22402]:      params)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/base.py", line 1271, in _execute_clauseelement
Jan 26 21:01:11 beaker beaker-server[22402]:      return self.__execute_context(context)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/base.py", line 1302, in __execute_context
Jan 26 21:01:11 beaker beaker-server[22402]:      context.parameters[0], context=context)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/base.py", line 1401, in _cursor_execute
Jan 26 21:01:11 beaker beaker-server[22402]:      context)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/base.py", line 1394, in _cursor_execute
Jan 26 21:01:11 beaker beaker-server[22402]:      context)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/sqlalchemy/engine/default.py", line 299, in do_execute
Jan 26 21:01:11 beaker beaker-server[22402]:      cursor.execute(statement, parameters)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/MySQLdb/cursors.py", line 173, in execute
Jan 26 21:01:11 beaker beaker-server[22402]:      self.errorhandler(self, exc, value)
Jan 26 21:01:11 beaker beaker-server[22402]:    File "/usr/lib64/python2.6/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
Jan 26 21:01:11 beaker beaker-server[22402]:      raise errorclass, errorvalue
Jan 26 21:01:11 beaker beaker-server[22402]:  OperationalError: (OperationalError) (1048, "Column 'system_id' cannot be null") 'INSERT INTO key_value_int (system_id, key_id, key_value) VALUES (%s, %s, %s)' (None, 3L, '6')
Jan 26 21:01:11 beaker beaker-server[22402]: bkr.server.wsgi DEBUG Rolling back for 500 response

Expected results:


Additional info:

Comment 2 Amit Saha 2014-02-03 05:43:56 UTC
I am wondering about the correct fix for this. I see two options:

- Fail to import with an error that the system doesn't exit
- Create the system like we do for others and populate it with the key value.

Comment 3 Nick Coghlan 2014-02-03 06:24:28 UTC
For all of the CSV types other than "Systems" and "Systems (for modification)", failing with a 422 Unprocessable Entity for unknown FQDN entries would be ideal (and the same goes for unknown non-empty system IDs in the latter case).

However, due to the current limitations of the CSV import page, a 200-with-an-error-message response may be the best we can do for now (that's what is currently done to report CSV import failures and it isn't worth investing in changing that just to resolve this bug).

Also, ensure that the handling of missing FQDNs is tested for all of the other CSV import types. Implicitly creating missing systems is acceptable (it isn't worth the hassle of deprecating that behaviour in cases where it already happens), but any other cases where it currently triggers 500 errors should also be updated to report a proper error message.

Comment 4 Amit Saha 2014-02-03 10:15:32 UTC
It turns out that the CSV import for non-existent systems creates the system if it doesn't exist for all the other import types: labinfo, power, install, exclude, groups barring keyvalue. It seems to me that there may have been an oversight in the key value import code.

So, I have taken the route of fixing this oversight rather than barring it, so that it is consistent with the others.

Preliminary patch: http://gerrit.beaker-project.org/#/c/2759/1

Comment 5 Nick Coghlan 2014-02-04 00:17:06 UTC
OK, after reviewing Amit's patch, it's (much) clearer that the original intent of the CSV import process was to always implicitly create the system if it wasn't already there, regardless of the kind of CSV record involved.

While I still have reservations about that design choice (although it does have the benefit of making a full import of a particular system independent of the order of the rows), I agree with Amit that the correct solution at this point in time is to simply bring the Key/Value import into line with the handling of other system rows, rather than moving to an approach where there is a "primary" system row type that must be provided first in order to create a new system.

Comment 8 Dan Callaghan 2014-02-18 01:38:44 UTC
Beaker 0.15.4 has been released.


Note You need to log in before you can comment on or make changes to this bug.