algocfg: Add relay profile, algocfg profile cmd #5069

Eric-Warehime · 2023-01-27T06:37:34Z

Summary

Adds a new config from which algocfg and set defaults separately from the Local defaults.

Some things that are troublesome w/ the way we use tags for config values.

Some code duplication required because we're using reflection a lot
Versioning on relay configs conflicts with Local defaults--this should be fine since non-specified values will be updated to the most recent config version.

The new commands:

> algocfg profile list
relay default
> algocfg profile set relay -d ~/.algorand/
> cat ~/.algorand/config.json
{
	"Version": 0,
	"Archival": true,
	"NetAddress": "4160",
	"EnableLedgerService": true
}

codecov · 2023-01-27T07:02:12Z

Codecov Report

Merging #5069 (72de1f2) into master (2c0a8db) will decrease coverage by 0.05%.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##           master    #5069      +/-   ##
==========================================
- Coverage   53.44%   53.39%   -0.05%     
==========================================
  Files         431      432       +1     
  Lines       54369    54409      +40     
==========================================
- Hits        29057    29052       -5     
- Misses      23053    23099      +46     
+ Partials     2259     2258       -1

Impacted Files	Coverage Δ
cmd/algocfg/profileCommand.go	`10.00% <10.00%> (ø)`
network/wsPeer.go	`65.97% <0.00%> (-1.84%)`	⬇️
catchup/peerSelector.go	`98.95% <0.00%> (-1.05%)`	⬇️
catchup/service.go	`68.94% <0.00%> (-0.95%)`	⬇️
network/wsNetwork.go	`64.98% <0.00%> (-0.19%)`	⬇️
ledger/tracker.go	`75.10% <0.00%> (ø)`
ledger/acctonline.go	`79.16% <0.00%> (+0.52%)`	⬆️
agreement/proposalManager.go	`98.03% <0.00%> (+1.96%)`	⬆️
agreement/cryptoVerifier.go	`69.71% <0.00%> (+2.11%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

cmd/algocfg/profileCommand.go

winder · 2023-01-31T20:29:41Z

cmd/algocfg/profileCommand.go

+			if err != nil {
+				reportErrorf("%v", err)
+			}
+			file := filepath.Join(dataDir, config.ConfigFilename)


Could you add a A config.json file already exists for this data directory. Would you like to overwrite it? (Y/n) prompt in case there is already a config file?

I wish it had a "-y"/"--yes" or "-f"/"--force" option to skip the prompt, or "-w" to overwrite. New to the problem I'm not strongly opinionated on which one feels right. Maybe "-f".
ALSO, I wish it was an update not an overwrite. A "default" to reset things makes sense as one option, but "relay" could just set the 4 fields it wants to set on top of an existing config.json

--yes looks required to use in shell scripts

cmd/algocfg/profileCommand.go

algorandskiy · 2023-02-09T20:19:34Z

cmd/algocfg/profileCommand.go

+			if err != nil {
+				reportErrorf("%v", err)
+			}
+			file := filepath.Join(dataDir, config.ConfigFilename)


--yes looks required to use in shell scripts

algorandskiy · 2023-02-09T20:22:55Z

cmd/algocfg/profileCommand.go

+					return
+				}
+			}
+			err = codecs.SaveNonDefaultValuesToFile(file, cfg, config.GetDefaultLocal(), nil, true)


nit: improvement idea: implement SaveNonDefaultValuesToWriter that works with not a file but a stream (and SaveNonDefaultValuesToFile just calls it) so that list profiles could also print non-default values in this profile.
Or allow outputting to stdout instead of config.json

That's a good idea, let me add that in a separate PR.

bbroder-algo · 2023-02-09T20:25:43Z

cmd/algocfg/profileCommand.go

+			if _, err := os.Stat(file); err == nil {
+				fmt.Printf("A config.json file already exists for this data directory. Would you like to overwrite it? (Y/n)")
+				reader := bufio.NewReader(os.Stdin)
+				resp, err := reader.ReadString('\n')


maybe ReadRune

bbroder-algo · 2023-02-09T20:29:53Z

cmd/algocfg/profileCommand.go

+
+func getConfigForArg(configType string) (config.Local, error) {
+	cfg := config.GetDefaultLocal()
+	switch configType {


maybe store the config generator function along with the array of profile names as opposed to this switch statement that will get appended to

Co-authored-by: Will Winder <wwinder.unh@gmail.com>

Eric-Warehime · 2023-02-10T17:58:35Z

Updated behavior

> cat ~/.algorand/config.json
{
	"Archival": true,
	"NetAddress": "4160",
	"EnableLedgerService": true,
	"EnableBlockService": true
}
> algocfg profile set default -d ~/.algorand --yes
> cat ~/.algorand/config.json
{
}

algorandskiy

no stdout support but lgtm

Eric-Warehime added the Enhancement label Jan 27, 2023

Eric-Warehime requested a review from winder January 27, 2023 06:37

winder requested review from algolucky and onetechnical January 27, 2023 18:30

winder reviewed Jan 27, 2023

View reviewed changes

cmd/algocfg/profileCommand.go Outdated Show resolved Hide resolved

winder mentioned this pull request Jan 30, 2023

goal: allow ConfigJSONOverride in local network templates #5017

Merged

winder reviewed Jan 31, 2023

View reviewed changes

Eric-Warehime marked this pull request as ready for review February 2, 2023 17:23

Eric-Warehime added 5 commits February 2, 2023 09:56

Add relay profile, algocfg profile cmd

d001725

Refactor

e964574

Update default

eb7081d

Add warning/confirmation when overwriting existing config

32256e8

merge upstream

1d0d604

Eric-Warehime force-pushed the algocfg-profiles branch from 7e5e17c to 1d0d604 Compare February 2, 2023 17:58

algorandskiy requested a review from jasonpaulos February 3, 2023 18:18

winder reviewed Feb 3, 2023

View reviewed changes

cmd/algocfg/profileCommand.go Outdated Show resolved Hide resolved

algorandskiy reviewed Feb 9, 2023

View reviewed changes

bbroder-algo reviewed Feb 9, 2023

View reviewed changes

Update cmd/algocfg/profileCommand.go

916d613

Co-authored-by: Will Winder <wwinder.unh@gmail.com>

Add --yes, refactor profile updaters

72de1f2

Eric-Warehime requested review from winder, algorandskiy and bbroder-algo and removed request for algolucky February 10, 2023 20:55

algorandskiy approved these changes Feb 10, 2023

View reviewed changes

winder approved these changes Feb 27, 2023

View reviewed changes

winder merged commit a525804 into algorand:master Feb 27, 2023

onetechnical mentioned this pull request Mar 1, 2023

DevOps: alphanet master remerge #5173

Merged

This was referenced Mar 10, 2023

go-algorand 3.15.0-beta Release PR #5190

Closed

go-algorand 3.15.0-beta Release PR #5194

Merged

Algo-devops-service mentioned this pull request Mar 17, 2023

go-algorand 3.15.0-stable Release PR #5218

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

algocfg: Add relay profile, algocfg profile cmd #5069

algocfg: Add relay profile, algocfg profile cmd #5069

Eric-Warehime commented Jan 27, 2023

codecov bot commented Jan 27, 2023 •

edited

Loading

winder Jan 31, 2023

brianolson Feb 3, 2023

algorandskiy Feb 9, 2023

algorandskiy Feb 9, 2023

algorandskiy Feb 9, 2023

Eric-Warehime Feb 10, 2023

bbroder-algo Feb 9, 2023

bbroder-algo Feb 9, 2023

Eric-Warehime commented Feb 10, 2023

algorandskiy left a comment

algocfg: Add relay profile, algocfg profile cmd #5069

algocfg: Add relay profile, algocfg profile cmd #5069

Conversation

Eric-Warehime commented Jan 27, 2023

Summary

codecov bot commented Jan 27, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eric-Warehime commented Feb 10, 2023

algorandskiy left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 27, 2023 •

edited

Loading