check_connection() usage

[PietrH]

check_connection() is currently called right before connect_to_etn(); check_connection() checks if the input arguments have the right form, then it attempts a database connection and checks it's class.

However, if we are making a database connection anyway, shouldn't these checks be a part of connect_to_etn()? Currently we are making two connection calls per function, this seems unnecessary.

Related, list_animal_ids() is using it's own similar checks instead of check_connection():

stopifnot(is.list(con))
stopifnot(any(names(con) == c("username", "password")))

There might be other functions using their own checks instead.

Cleaning up

I see 2 scenarios:

A) Keep using check_connection() in hosting functions

• call check_connection() in list_animal_ids() and remove the then redundant checks
• Pay extra attention check_connection() is called when making a connection

2 calls per function, this takes a bit longer.

B) Move check_connection inside connect_to_etn

• implement check_connection() in connect_to_etn()
• remove check_connection() from function internals

This option would add an extra step into the translation of local database functions to API hosting functions. Possibly causing extra bugs. But it's quicker.

Questions

• Is there a reason check_connection() is separate from connect_to_etn()?
• Is the time gain, loss of complexity worth moving check_connection() inside connect_to_etn(), and the accompanying code changes worth the risk of extra bugs at this stage?
• What is the impact on error reporting on placing check_connection() inside connect_to_etn()?
• Do the current tests still work if we move check_connection() inside connect_to_etn()?

[PietrH]

We can go ahead with option A) and still implement option B) later

[PietrH]

If we adapt check_connection() to use an existing connection, then connect_to_etn() needs to come first, and connect_to_etn() needs to report errors if the connection failed to be created: wrong password, input object doesn't contain username and password, ...

[PietrH]

I've adapted check_connection() to only check the connection on the live-test branch, thus it needs to be called after connect_to_etn()

[PietrH]

Implemented in #1