Creating a restart function for storage classes to fix broken database connections

[rich-hart]

Fix #47

[rich-hart]

Just curious if there is anyway to make this section more concise without having to use these nested try / except statements

[karenc]

I don't know a better way to do this.

[coveralls]

Coverage increased (+1.22%) to 97.32% when pulling f9d4ff3 on database_restart into 737a36c on master.

[coveralls]

Coverage increased (+1.22%) to 97.32% when pulling f9d4ff3 on database_restart into 737a36c on master.

[coveralls]

Coverage increased (+1.22%) to 97.32% when pulling bf4d85e on database_restart into 737a36c on master.

[rich-hart]

Since were adding a coveralls configuration file I think that it would be best to leave this pull request as two separate commits since the commits address two distinct issues.

[rich-hart]

adding a variable to store the db_connection string needed to restart the database when a connection is broken or lost.

[karenc]

Come to think about it, shouldn't your note be comment in the code?

[coveralls]

Changes Unknown when pulling 333bcf1 on database_restart into * on master*.

[karenc]

I don't think this comment is relevant

[coveralls]

Changes Unknown when pulling b06530a on database_restart into * on master*.

[coveralls]

Changes Unknown when pulling 8bca86d on database_restart into * on master*.

[mmulich]

Why is the storage_managment function in the views module? I know it wasn't put in there during this pull request. But it is definitely in the wrong module.

Otherwise, I think this is ready to roll.

[coveralls]

Changes Unknown when pulling 1b2446a on database_restart into * on master*.

[mmulich]

I adjusted the .coveragerc file. The coverage run has now been scoped to the cnxauthoring package (Note, coveralls does this by default). The report has also been limited to include all the modules and omit the tests and scripts.

[karenc]

Oh but may be it's good to include the tests in the coverage. Just in case tests got skipped because we have some indentation error, or because of copy and paste error (See openstax/cnx-archive@09fdd6a).

[mmulich]

@karenc That's a good point, but it also artificially inflates the coverage (i.e. 92% vs 96%). I'll pull it out in a followup pull request. Unfortunately, you comment came in just after I pushed the button.

[rich-hart]

@pumazi the storage_managment function was added to views.py when 6c7025d was committed. It was used to clean up the 'persist' and 'abort' functions that were scattered throughout the views module and to also allow for a consistent way of sending an httpexceptions to the user. I left the storage_managment in views because that where the persist and abort functions were currently being used. Also, as karen pointed out when I was working on it, generating storage failure messages and passing them to the user is essentially a responsibility for views. I see your point though because I was originally thinking the same thing but does my explanation make sense? Let me know if you still think moving somewhere else would be a better practice

[mmulich]

I believe it would be better to make the BaseStorage class a context manager, then this second function wouldn't be needed. Simply import storage and use it. But that's another story altogether...

For now, I'd say it belongs in the cnxauthoring.storage.main and importable from cnxauthoring.storage.

[rich-hart]

ok! I made an issue #200 suggesting that we make a context manager in the Base storage class. I'll also make a pull request to refactor the storage_management decorator.

[mmulich]

I may have misunderstood this somewhat. But we'll see what happens.