Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce generic/central SQL logging #35

Open
simonkelly1973 opened this issue Mar 13, 2020 · 4 comments
Open

Introduce generic/central SQL logging #35

simonkelly1973 opened this issue Mar 13, 2020 · 4 comments

Comments

@simonkelly1973
Copy link
Member

There are various events currently written the local "demo-apps.log". As part of the data logger project, we also need to write those to a database.

simonkelly1973 added a commit that referenced this issue Mar 13, 2020
@simonkelly1973
Copy link
Member Author

e78fbc4 introduces a SQLLogging class. In the fullness of time, when we are doing more and more datalogging, that should provide a JDBC connection pool, and methods to get a connection, so that all the other places in the code don't need to have code to read the connection username,password etc.

For the time being, it just provides a very simple static log message that takes a class name and a message, and puts it into the "devicelogs" table in the database.

AbstractDevice makes use of that to log the changes to limits. AbstractDevice also changes the way it stores floats for samples. In fact, this should be refactored further, so that AbstractDevice is not doing any connection work, instead getting the connection via SQLLogging.

@simonkelly1973
Copy link
Member Author

10e7a22 makes one important change and does a few other things

The SQLLogging class in com.agitar.common provides a getConnection() method. This makes it simpler for all other locations that need a connection to obtain one, without reading icejdbc.properties. This also means that only com.agitar.common needs to have the mariadb (or any other) JDBC driver on its classpath.

IceApplicationProvider.IceApp specified several state changing functions - in particular, stop and destroy. The comment for "stop" is

"Temporary passivate the component. The component is "put on ice" with the potential restart request to follow. No expensive resources are excepted to be freed at this point."

The comment for "destroy" is

Destructor for the application. resources should be freed, state cleared. app is not expected to be functional once it had been destroyed.

A number of previous implementations of IceApp call stop on the relevant application from both the stop and destroy methods from the corresponding factory for the application. However, since the JDBC connection is considered to be an "expensive resource", then in the places that use them, a destroy() method has been implemented, and the connection is closed in those methods. In fact there is no actual use of the "stop" method to "passivate the component" except alongside the "destroy" method in IceAppsContainer around line 370 where the main window is being closed (i.e. the application is exiting). Perhaps the intents and use of those methods should be reviewed.

@simonkelly1973
Copy link
Member Author

0df8735 should have been included in 10e7a22 (and was a real functionality breaker!). It includes the reference for the MariaDB JDBC driver into the common class.

@simonkelly1973
Copy link
Member Author

b4a1085 introduces "placebo" connections. The naming is a dreadful joke of course, but the idea is just that it's a dummy connection that can be used when the user does not have an icejdbc.properties file (because they don't have a database). Instead of constantly needing to check in all locations whether there is a database in place, we just provide this dummy connection instead.

PlaceboConnection is just an implementation of java.sql.Connection, with almost everything nulled out or just left at the default values in Eclipse. The only method that's actually implemented is "prepareStatement", which returns a PlaceboPreparedStatment.

In turn, PlaceboPreparedStatement is just an empty implementation of PreparedStatement. The only non default thing in there is returning true for execute().

Back in SQLLogging, we just check if there is a properties file or not, and if not then we use the placebo instead.

One future enhancement could be to use the placebo in the case where there is a properties file for a valid database, but connection creation fails for some reason - i.e. we treat the placebo as a fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant