Push docker image to registry#7
Conversation
gregcusack
force-pushed
the
push-docker-to-registry
branch
from
9d1e086 to
c9c94a3
Compare
gregcusack
force-pushed
the
push-docker-to-registry
branch
2 times, most recently
from
1721ca3 to
e5097ad
Compare
gregcusack
force-pushed
the
push-docker-to-registry
branch
from
e5097ad to
ef82a2c
Compare
gregcusack
force-pushed
the
push-docker-to-registry
branch
from
69e7e75 to
d1e0aaa
Compare
gregcusack
force-pushed
the
push-docker-to-registry
branch
from
d1e0aaa to
ef5146a
Compare
yihau
left a comment
yihau
left a comment
There was a problem hiding this comment.
sorry for the super late reviewing 😞
no worries! lots of other stuff going on rn. thank you! |
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looking great! Just a few things, and thanks for addressing the previous points
| let output = match Command::new("sh") | ||
| .arg("-c") | ||
| .arg(&command) | ||
| .stdout(Stdio::null()) | ||
| .stderr(Stdio::null()) | ||
| .spawn() | ||
| .expect("Failed to execute command") | ||
| .wait_with_output() | ||
| { | ||
| Ok(res) => Ok(res), | ||
| Err(err) => Err(Box::new(err) as Box<dyn Error>), | ||
| }?; |
There was a problem hiding this comment.
nit: you should be able to simplify this to something like:
let output = match Command::new("sh")
.arg("-c")
.arg(&command)
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.expect("Failed to execute command")
.wait_with_output()
.map_err(Box::new)?;
with maybe some trickery on the Box::new bit
| let programs = vec![ | ||
| ( | ||
| "token", | ||
| "3.5.0", | ||
| "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA", | ||
| "BPFLoader2111111111111111111111111111111111", | ||
| ), | ||
| ( | ||
| "token-2022", | ||
| "0.9.0", | ||
| "TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb", | ||
| UPGRADEABLE_LOADER, | ||
| ), | ||
| ( | ||
| "associated-token-account", | ||
| "1.1.2", | ||
| "ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL", | ||
| "BPFLoader2111111111111111111111111111111111", | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
These seem to be missing memo v1, memo v3, and feature-proposal: https://github.com/anza-xyz/agave/blob/deec1f450d1f2eae40158f481a37f1e8be79ca03/fetch-spl.sh#L49
Was that intentional?
There was a problem hiding this comment.
ahhhh shoot. i had some url issues with these. and took them out but forgot to put them back in once i figured out the issue. smh. will fix
| ), | ||
| ]; | ||
|
|
||
| for (name, version, address, loader) in programs { |
There was a problem hiding this comment.
nit: past two fields, it's typically clearer to create a new struct, like
struct GenesisProgram {
name: &str,
version: &str,
address: &str,
loader: &str,
}
| .long("slots-per-epoch") | ||
| .takes_value(true) | ||
| .help("override the number of slots in an epoch"), | ||
| .help("override the number of slots in an epoch. Default for cluster_type: development -> 8192. |
There was a problem hiding this comment.
Or crap, I didn't realize it was still 8192 for dev clusters. Might be worth revisiting in separate work
| .long("target-lamports-per-signature") | ||
| .takes_value(true) | ||
| .help("Genesis config. target lamports per signature"), | ||
| .help("Genesis config. target lamports per signature. Default: 10000"), |
There was a problem hiding this comment.
Gosh, I'm learning a lot through these PRs. I can't believe this concept still exists!
3 participants
Summary of Changes
DockerConfigstruct and add inDockerImagestruct7th PR in a series of PRs that will build out the mongon testing framework for deploying validator clusters on Kubernetes
Previous PRs: