[rest] Adding SKEMA_GRAPH_DB_HOST and SKEMA_GRAPH_DB_PORT environment variables

[vincentraymond-ua]

Adding environmental variables

Adds the SKEMA_GRAPH_DB_HOST and SKEMA_GRAPH_DB_PORT environmental variables to skema/rest.

Updates the execution engine to use these values to connect to Memgraph.

See the corresponding PR to set these values in the ASKEM-TA1-DockerVM repo:
ml4ai/ASKEM-TA1-DockerVM#44

[Free-Quarks]

LGTM!

[myedibleenso]

@vincentraymond-ua , this is a great start, but we will also need to modify how the skema-rs REST API receives this information:

skema/skema/skema-rs/skema/src/bin/skema_service.rs

Lines 19 to 21 in d193a52

	/// Database host
	#[arg(long, default_value_t = String::from("localhost"))]
	db_host: String,

It seems it uses as assumed port.

[Free-Quarks]

The information is passed as a command line argument on the docker side entrypoint for the RUST service. So this should still work for both services, since the environment variable is also set on the docker side.

[myedibleenso]

The information is passed as a command line argument on the docker side entrypoint for RUST service. So this should still work for both services, since the environment variable is also set on the docker side.

@Free-Quarks, can you explain what happens today if you're using something other than the default DB port (ex. a proxied request)?

[Free-Quarks]

I agree it should be an environment variable and is a left over way of doing things before we dockerized everything

[myedibleenso]

If I'm reading the Rust code correctly (big if), if you include the port in your value for db_host it will be used by execute_query:

skema/skema/skema-rs/skema/src/database.rs

Lines 89 to 94 in d193a52

	pub fn execute_query(query: &str, host: &str) -> Result<(), MgError> {
	// Connect to Memgraph.
	let connect_params = ConnectParams {
	host: Some(host.to_string()),
	..Default::default()
	};

Is that correct, @Free-Quarks ?

[myedibleenso]

Actually, I take that back. I think a port is assumed: https://docs.rs/rsmgclient/latest/rsmgclient/struct.ConnectParams.html

Could we at least optionally specify the db port in the skema-rs runnable with the standard bolt-like default?

[myedibleenso]

let connect_params = ConnectParams { 
         host: Some(host.to_string()), 
         port: Some(port.to_int()),
         ..Default::default() 
     }; 

... and:

#[derive(Debug)]
   pub struct Config {
      pub db_host: String,
      // FIXME: how to set a default (ex. 7687) here?
      pub db_port: int
   }

[myedibleenso]

^ idk if my Rust is correct (probably not), but hopefully it at least conveys the idea.

[Free-Quarks]

Yeah I think I get what you are saying. (looping in @adarshp to double check as he wrote the code) We have it setup to get the port from the command line argument, but when we hit Memgraph we don't specify it each time and assume it is static, but we could specify it in the way you are showing. I could add that to this PR.

[myedibleenso]

I see use of host in a bunch of places, so probably doing this would involve touching many lines of code...

[myedibleenso]

I will deal with it on the deployment side and drop pushing for a change for now...

[myedibleenso]

Just noting that a DB port cannot currently be passed on the Rust side.

[myedibleenso]

@vincentraymond-ua , do you these env variables need to be noted in our documentation, or is it sufficient to update the dockerVM repo?

[vincentraymond-ua]

@myedibleenso - Thats a good question, it may be valuable to have these described in the documentation, perhaps in the dev environment page: https://ml4ai.github.io/skema/dev/env/