error 500 when adding a MW with duplicated ID

[italovalcy]

Hi,

When adding a MW with duplicated ID, Kytos server returns a 500. error (Error 500 when creating a MW with same id of existing one):

Feb  6 20:53:31 130c32ca043b kytos.core.controller:ERROR app:1449:  Exception on /api/kytos/maintenance/v1 [POST]#012Traceback (most recent call last):#012  File "/usr/local/lib/python3.9/dist-packages/flask/app.py", line 2073, in wsgi_app#012    response = self.full_dispatch_request()#012  File "/usr/local/lib/python3.9/dist-packages/flask/app.py", line 1519, in full_dispatch_request#012    rv = self.handle_user_exception(e)#012  File "/usr/local/lib/python3.9/dist-packages/flask_cors/extension.py", line 165, in wrapped_function#012    return cors_after_request(app.make_response(f(*args, **kwargs)))#012  File "/usr/local/lib/python3.9/dist-packages/flask/app.py", line 1517, in full_dispatch_request#012    rv = self.dispatch_request()#012  File "/usr/local/lib/python3.9/dist-packages/flask/app.py", line 1503, in dispatch_request#012    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)#012  File "//var/lib/kytos/napps/kytos/maintenance/main.py", line 108, in create_mw#012    self.scheduler.add(maintenance, force=force)#012  File "//var/lib/kytos/napps/../napps/kytos/maintenance/models.py", line 320, in add#012    self.db_controller.insert_window(window)#012  File "/usr/local/lib/python3.9/dist-packages/tenacity/__init__.py", line 324, in wrapped_f#012    return self(f, *args, **kw)#012  File "/usr/local/lib/python3.9/dist-packages/tenacity/__init__.py", line 404, in __call__#012    do = self.iter(retry_state=retry_state)#012  File "/usr/local/lib/python3.9/dist-packages/tenacity/__init__.py", line 349, in iter#012    return fut.result()#012  File "/usr/lib/python3.9/concurrent/futures/_base.py", line 433, in result#012    return self.__get_result()#012  File "/usr/lib/python3.9/concurrent/futures/_base.py", line 389, in __get_result#012    raise self._exception#012  File "/usr/local/lib/python3.9/dist-packages/tenacity/__init__.py", line 407, in __call__#012    result = fn(*args, **kwargs)#012  File "/usr/local/lib/python3.9/dist-packages/kytos/core/retry.py", line 25, in decorated#012    return func(*args, **kwargs)#012  File "//var/lib/kytos/napps/../napps/kytos/maintenance/controllers/__init__.py", line 64, in insert_window#012    self.windows.insert_one({#012  File "/usr/local/lib/python3.9/dist-packages/elasticapm/instrumentation/packages/base.py", line 210, in call_if_sampling#012    return self.call(module, method, wrapped, instance, args, kwargs)#012  File "/usr/local/lib/python3.9/dist-packages/elasticapm/instrumentation/packages/pymongo.py", line 94, in call#012    return wrapped(*args, **kwargs)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/collection.py", line 606, in insert_one#012    self._insert_one(#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/collection.py", line 547, in _insert_one#012    self.__database.client._retryable_write(acknowledged, _insert_command, session)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/mongo_client.py", line 1399, in _retryable_write#012    return self._retry_with_session(retryable, func, s, None)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/mongo_client.py", line 1286, in _retry_with_session#012    return self._retry_internal(retryable, func, session, bulk)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/mongo_client.py", line 1320, in _retry_internal#012    return func(session, sock_info, retryable)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/collection.py", line 545, in _insert_command#012    _check_write_command_response(result)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/helpers.py", line 216, in _check_write_command_response#012    _raise_last_write_error(write_errors)#012  File "/usr/local/lib/python3.9/dist-packages/pymongo/helpers.py", line 188, in _raise_last_write_error#012    raise DuplicateKeyError(error.get("errmsg"), 11000, error)#012pymongo.errors.DuplicateKeyError: E11000 duplicate key error collection: napps.maintenance.windows index: id_1 dup key: { id: "28bcd26b963040eabf0007e27deec683" }, full error: {'index': 0, 'code': 11000, 'keyPattern': {'id': 1}, 'keyValue': {'id': '28bcd26b963040eabf0007e27deec683'}, 'errmsg': 'E11000 duplicate key error collection: napps.maintenance.windows index: id_1 dup key: { id: "28bcd26b963040eabf0007e27deec683" }'}

Should we accept ID as the input attribute? It looks like ID would be an internal field - auto-generated (as it currently is).

If we accept ID as an input parameter, it would be nice to have some validations and return a 4xx error instead of 500. But once again, I don't think we should allow ID to be received from the user.

[viniarck]

Great catch Italo. From what I can tell historically, this settable indexed id was probably to act like an unique (meaningful) name.

I think we could also not expose it to be set, fewer validations and code to maintain as well. If we end up not allowing it to be set, we can also end up in cases with a random ID and empty description (if users don't fill out), which is OK as well, but in the front-end having a tool tip or something encouraging to always add something in the description might be useful (I'm thinking of cases where a network operator for instance create multiple non overlapping MWs, all relatively similar but without description and then don't remember which one was for what purpose).

@Ktmi if you could add this one in your radar and assess/propose a solution and fix it, thanks.

[Ktmi]

This sounds like its going to need an API version bump. We can combine this with removing the timezone requirement for datetime.

[viniarck]

This sounds like its going to need an API version bump. We can combine this with removing the timezone requirement for datetime.

Indeed, David. Regarding removing the timezone, ideally, let's also ship a migration script with readme instructions to update the values of existing documents accordingly. Here's a related script example that we've shipped before.

Thanks.

[Ktmi]

Closed with #78