Skip to content

Auto avalanchego #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Auto avalanchego #30

merged 1 commit into from
Aug 4, 2022

Conversation

morrisettathena
Copy link
Contributor

@morrisettathena morrisettathena commented Jul 23, 2022

The e2e test now installs avalanchego via https://docs.rs/crate/avalanche-installer/0.0.3.

Copy link
Contributor

@hexfusion hexfusion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to replacing bash with rust :). Added a few notes.

let (mut exec_path, is_set) = get_avalanchego_path();
if !is_set {
info!("avalanchego_path not set, installing to temp");
let (path, _) = avalanche_installer::avalanchego::download(None, None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

questions: download grabs the latest release[1]. seems like we would want to pin this to an explicit version compatible with the mini-kvvm and or have a test against latest download_version ? Also would it make sense to pass the params to download` since we have them so we can run this in CI with darwin?

Finally if this test is run locally and in general it would be good to validate what version the binary is and print it. Perhaps the path would be useful for this?

[1] https://github.com/gyuho/avalanche-installer/blob/main/src/avalanchego.rs#L84

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, I can modify this code to follow this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, after reviewing these a little bit:

  • I don't currently know of a way in the avalanchego installer to install a specific version. Maybe this is a feature we could add to avalanchego installer? The only other way would be to re-incorporate it in the bash script, which sort of defeats the purpose of what the installer is for.
  • I also don't know if it entirely makes sense to pass params to download, because download automatically does the work of determining what os/arch is needed for the system.
  • When you say have a test against latest download_version do you mean having two tests, one that automatically installs the latest version and another that fetches avalanchego from the local machine?
  • added a little bit of logic to print out the version that avalanchego is running

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes lets talk to @gyuho on his thoughts

@hexfusion
Copy link
Contributor

can we rebase these commits into a few which are reflective of the actual changes.

@hexfusion hexfusion added this to the v0.0.1 milestone Jul 28, 2022
@morrisettathena
Copy link
Contributor Author

Now this installs avalanchego version "v1.7.16" for every e2e test

@hexfusion hexfusion merged commit e99b09b into main Aug 4, 2022
@hexfusion hexfusion deleted the auto_avalanchego branch August 4, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants