Nats: db proposal #42
Conversation
steveej
force-pushed
the
nats-baseline
branch
3 times, most recently
from
382ddd1 to
78492d1
Compare
JettTech
requested review from
zeeshan595 and
a team
and removed request for
zeeshan595
| // let mongo_uri = get_mongodb_url(); | ||
| // let client_options = ClientOptions::parse(mongo_uri).await?; | ||
| // let client = MongoDBClient::with_options(client_options)?; | ||
| let mongo_uri = get_mongodb_url(); |
There was a problem hiding this comment.
Should we really be calling into MongoDB from the host_agent code? The agent should communicate with the services running centrally, which in turn is the only touch point to the central MongoDB instance. All communication from the agent goes through the API, which is now using NATS as a transport and won't go direct to things like MongoDB.
There was a problem hiding this comment.
You're 100% correct. This was added here for the convenience of testing in the first iteration. The host agents will not have a mongodb instance in production. I actually already have a PR out that both adds the workload api for the orchestrator and separates out the api for the orchestrator and host such that only the orchestrator service api has access to mongodb.
| @@ -139,6 +156,11 @@ where | |||
| } | |||
| } | |||
|
|
|||
| // Helpers: | |||
| pub fn get_mongodb_url() -> String { | |||
| std::env::var("MONGO_URI").unwrap_or_else(|_| "mongodb://127.0.0.1:27017".to_string()) | |||
There was a problem hiding this comment.
This doesn't seem right either. We won't be running a MongoDB instance on the holoports and other workload hosts.
There was a problem hiding this comment.
Agreed. See above. :)
| @@ -245,8 +267,18 @@ mod tests { | |||
|
|
|||
| fn get_mock_host() -> schemas::Host { | |||
There was a problem hiding this comment.
Is this a placeholder for the inventory? We can probably replace it soon with a call to the hpos_hal::inventory module. The schema itself is still under some change, but that's OK.
There was a problem hiding this comment.
The Host struct that this test is referencing is schema for the Host in mongodb. It is intended to store the relevant inventory data that the hal::inventory module generates. I agree that we could start to align the two structs, even if the hal::inventory is subject to change. I'll take a look into aligning those this week. 👍
Updates:
callmethod