API Behaviour: authentication and switching between local db and API

[PietrH]

This is a technical issue to capture a discussion regarding the future migration of ETN to using an OpenCPU API.

Function Naming

• Functions will retain the same names, and no exported duplicate functions will be made.

• The functions will allow users to switch between using the API, or a local database connection (as they do now on the RStudio Server instance).

Authentication and switching between the API and a local database connection

flowchart TD

main(("list_animal_ids(
    other_arguments,
    connection = NULL,
    api = TRUE
    )")) --api=true--> api_helper(("list_animal_ids_api(
    other_arguments
    )"))
main --api=false--> sql_helper(("list_animal_ids_sql(
other_arguments
)"))
main --"lifecycle::is_present(connection)"-->warning[["warning: connection is depreciated,
please set API=FALSE to use local db"]]
warning --> sql_helper
api_helper --> get_credentials1["credentials = get_credentials()"]
api_helper --> other_arguments1["other_arguments"]
get_credentials1 --> etnservice[/"ETNSERVICE"/]
other_arguments1 --> etnservice
sql_helper --> get_credentials2["credentials = get_credentials()"]
get_credentials2 --> connection
sql_helper --> other_arguments2["other_arguments"]
connection --> code>"existing code"]
other_arguments2 --> code

• Authentication via the API uses an internal helper get_credentials() that gets the username/password from the environment (or later on, asks the user to fill them in, warns for missing/empty pwd, uses keyring similar to move2)

• Users will be able to pass a connection argument as they do now, and if they do so, ETN will try to use a local database connection. Old scripts will continue to work on the RStudio server. However, the contents of the connection argument are unused, as the SQL helper will use get_credentials() to build a connection argument for itself internally.

• If a user wants to run an old script locally, using the API, all they need to do is remove the connection argument.

• If a user wants to use the local db, they set API = FALSE

• The API and local db helpers both use a credentials argument populated with get_credentials().

• main can be merged into this script that contains both helpers, the changes to main need to be merged manually into the sql helper

• This allows for switching between api and local db use, allows for the reuse of existing scripts when run on the RStudio Server, allows adaptation of existing scripts with minimal effort (as no authentication argument needs to be added for functions to work)

• Currently all functions require authentication, but in the future we can consider disabling authentication for functions that report data without an embargo: this is a database level change

Idea's

• [ ] Use etnservice as a dependency and drop _sql helpers
• [x] Use a generic helper to communicate with the api
• [x] Use a generic helper to switch between using the api and the sql helper

[PietrH]

Should the tests for the API only, or once for the API route, and once for the SQL route?

For integration into github actions, only the API route makes sense.

[PietrH]

get_credentials() now prompts the user when no values are stored in sys.env: d1a2b06c11f0214e5855f97750db53fdef5bac95

[PietrH]

I'm now using a generic api helper instead of a api helper per function: 8d9a001b108073ac6c9ee93048e0a81248a9e3a3

[PietrH]

The code used for the main functions is very repetitive, and suitable for wrapping in a helper. This would make changes to this code easier in the future and would keep it DRY.

However, it currently is immediately apparent what the main function is doing (some parsing, and passing arguments on to two helpers, one general, and one specific). Using a helper to do this, would detract from the functions being self explaining.

[PietrH]

Instead of having a helper function with almost the same code as etnservice, wouldn't it be cool if the helper could instead just get the code from etnservice directly and evaluate it in place for it's sql functions?

That way we only have to maintain the functions in one place.

[peterdesmet]

Right, we currently need to maintain it in two places ... We could load etnservice as a dependency, but then we also bring in its dependencies.

[PietrH]

etnservice will, at least for the moment have the same dependencies as etn as they are both running the same sql functions. etn also adds some packages for handling the communication with the api.

I think loading etnservice as a dependency is the way forward, but for now I'll keep going ahead with a hard coded _sql helper. And migrate to using etnservice when we are bit further ahead.