Support deploying Arroyo to a nomad cluster #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jacksonrnewhouse
approved these changes
Apr 18, 2023
| let query = CompileQueryReq::decode(&*query).expect("Failed to decode query request"); | ||
|
|
||
| let resp = service.compile(query).await.unwrap(); | ||
| println!( |
There was a problem hiding this comment.
should this be an info!()?
There was a problem hiding this comment.
It's writing json to std out so that it can be consumed by whatever is calling it. If it were an info!, that would send it to the logging system instead.
| @@ -447,6 +448,18 @@ impl ControllerServer { | |||
| .create_pool(Some(deadpool_postgres::Runtime::Tokio1), NoTls) | |||
| .unwrap(); | |||
|
|
|||
| // test that the DB connection is valid | |||
| let _ = pool.get().await.unwrap_or_else(|e| { | |||
| panic!( | |||
There was a problem hiding this comment.
Is it standard to panic in these cases, rather than either returning an error or exiting?
There was a problem hiding this comment.
Panicing will produce an error and exit. Eventually we'll probably want clearer error handling for users, but while most of our users are going to be developers working on the system I think it's more helpful to have the context that a panic gives you (like the line).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a number of improvements that allow an arroyo cluster to be deployed to Nomad. There is a complementary docs PR (ArroyoSystems/arroyo-docs#7) and a new repo (https://github.com/ArroyoSystems/arroyo-nomad-pack) that contains the Nomad orchestration code.
The changes in this PR include:
S3_REGIONandS3_BUCKETnow control all usages of S3ADMIN_PORTandGRPC_PORTconfigurations, and this can be overridden for each services with{SERVICE}_{ADMIN, GRPC}_PORT, for exampleAPI_ADMIN_PORTThere is also a breaking change. In order to more seamlessly support using a single bucket for artifacts and checkpoints, checkpoint are now stored in
{BUCKET}/{job_id}/checkpoints. This will break existing jobs that expect checkpoints to be in the former location.