Skip to content
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 by Bors] - Add CLI tool to generate an identity file #3167

Closed
wants to merge 4 commits into from

Conversation

orgr
Copy link
Contributor

@orgr orgr commented Apr 11, 2022

Motivation

Closes #3157

Changes

Change the ensureIdentity to public EnsureIdentity,
Add a CLI tool that calls EnsureIdentity

DevOps Notes

  • This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
  • This PR does not affect public APIs
  • This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
  • This PR does not make changes to log messages (which monitoring infrastructure may rely on)

Copy link
Contributor

@dshulyak dshulyak left a comment

Choose a reason for hiding this comment

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

additionally we need to package it somehow. probably add it to the spacemesh docker image for simplicity

gen_identity/gen_identity.go Outdated Show resolved Hide resolved
gen_identity/gen_identity.go Outdated Show resolved Hide resolved
gen_identity/gen_identity.go Outdated Show resolved Hide resolved
gen_identity/gen_identity.go Outdated Show resolved Hide resolved
@@ -84,6 +84,7 @@ COPY . .
# And compile the project
Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw, docker build fails on the curl line 57 unless I add the -k flag (ignore certificates), but also I've been working from a WSL on a Windows machine so it could be a local issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

definitely something local, it works on the rest of prs and other dev environments

@orgr orgr requested a review from dshulyak April 15, 2022 15:18
Copy link
Contributor

@dshulyak dshulyak left a comment

Choose a reason for hiding this comment

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

i wonder what would be a better name for the binary. maybe genp2pid or more verbose gen-p2p-identity ?

gen_identity/gen_identity.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@@ -84,6 +84,7 @@ COPY . .
# And compile the project
Copy link
Contributor

Choose a reason for hiding this comment

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

definitely something local, it works on the rest of prs and other dev environments

hare p2p: get-libs
cd cmd/$@ ; go build -o $(BIN_DIR)go-$@$(EXE) $(GOTAGS) .
cd $@ ; go build -o $(BIN_DIR)go-$@$(EXE) $(GOTAGS) .
Copy link
Contributor

Choose a reason for hiding this comment

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

this actually needs to be removed, i forgot about it

@orgr orgr requested a review from countvonzero as a code owner April 25, 2022 12:22
@orgr orgr requested a review from dshulyak April 25, 2022 12:50
@dshulyak
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Apr 26, 2022
@bors
Copy link

bors bot commented Apr 26, 2022

try

Build failed:

@bors
Copy link

bors bot commented Apr 26, 2022

🔒 Permission denied

Existing reviewers: click here to make orgr a reviewer

@orgr
Copy link
Contributor Author

orgr commented Apr 26, 2022

@dshulyak, I've fixed the comments, linter should pass now

@dshulyak
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Apr 27, 2022
@bors
Copy link

bors bot commented Apr 27, 2022

try

Build failed:

@dshulyak
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Apr 27, 2022
@bors
Copy link

bors bot commented Apr 27, 2022

try

Build succeeded:

@dshulyak
Copy link
Contributor

bors merge

bors bot pushed a commit that referenced this pull request Apr 27, 2022
## Motivation
Closes #3157 

## Changes
Change the ensureIdentity to public EnsureIdentity,
Add a CLI tool that calls EnsureIdentity

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
@bors
Copy link

bors bot commented Apr 27, 2022

Build failed:

@dshulyak
Copy link
Contributor

new test is somewhat flaky. i will take a look

@dshulyak
Copy link
Contributor

bors merge

bors bot pushed a commit that referenced this pull request Apr 27, 2022
## Motivation
Closes #3157 

## Changes
Change the ensureIdentity to public EnsureIdentity,
Add a CLI tool that calls EnsureIdentity

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
@bors
Copy link

bors bot commented Apr 27, 2022

Build failed:

@dshulyak
Copy link
Contributor

bors merge

bors bot pushed a commit that referenced this pull request Apr 28, 2022
## Motivation
Closes #3157 

## Changes
Change the ensureIdentity to public EnsureIdentity,
Add a CLI tool that calls EnsureIdentity

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
@bors
Copy link

bors bot commented Apr 28, 2022

Pull request successfully merged into develop.

Build succeeded:

@bors bors bot changed the title Add CLI tool to generate an identity file [Merged by Bors] - Add CLI tool to generate an identity file Apr 28, 2022
@bors bors bot closed this Apr 28, 2022
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.

cmd, p2p: add command line tool to generate and save p2p identity
2 participants