detect if dcrcli.conf is not configured #64

ademuanthony · 2018-12-17T02:02:57Z

fixes #50
If the config file is not found and the user did not pass Dcrwallet credentials in command line options, propose to guide the user in creating a new config file and do same if the user accepts, else quit the application

The app will suggest default values and use them if none is provided for the RPC credential

barisere

You've done good work here. Let's tie up loose edges so we can merge this PR.

config.go

cli/terminalprompt/terminalprompt.go

config.go

cli/terminalprompt/terminalprompt.go

ademuanthony · 2018-12-21T02:40:17Z

Add a new handler; 'dcrcli configure' that will create the dcrcli.conf file for the user.

the file is now generated using the 'text/template' package and the sample config is now removed leaving the string holding the template as the single source of truth for the configuration file.

itswisdomagain

Getting this auto-config thing to work smoothly is not proving as straightforward as we'd like but we'll get it. Some more thoughts on what could be done better

cli/commands.go

cli/helpers.go

cli/handlers.go

config.go

main.go

ademuanthony · 2018-12-22T15:23:55Z

I have added an --init flag and moved the configure func to config.go.
The user can now set the config by calling 'dcrcli --init' or accepting the auto-create suggestion during startup

itswisdomagain

We're close! Minor changes left...

itswisdomagain · 2018-12-23T15:08:17Z

config.go

+	defaultRpcAddress  = "localhost:19111"
+	defaultRpcUsername = "rpcuser"
+	defaultRpcPassword = "rpcpass"
+	ServerAddress      = "127.0.0.1:7778"


Let's use defaultHTTPServerAddress instead, btw, ServerAddress doesn't need to be public. We can/should make defaultHTTPServerAddress = 127.0.0.1:7778

Also, we don't need to automatically save a value for serveraddress to dcrcli.conf. The program is designed such that if there is no value set in config, the default value would be used.

If we must save a value for this option to dcrcli.conf, we should request it from the user. But I don't think that is relevant, we probably should just stick to having a default value to use if user doesn't manually supply one.

itswisdomagain · 2018-12-23T15:15:49Z

config.go

@@ -129,18 +137,45 @@ func loadConfig(appName string) (*config, []string, error) {
 		os.Exit(0)
 	}

+	//config
+	if preCfg.Init {


What if the file already exists and user passes the --init flag? I think a warning is in order before the existing config file is overwritten.

itswisdomagain · 2018-12-23T15:20:58Z

config.go

 			parser.WriteHelp(os.Stderr)
 			return nil, nil, err
 		}
+
+		if preCfg.WalletRPCServer == "" || preCfg.RPCUser == "" || preCfg.RPCPassword == "" {


When I run dcrcli -c="localhost:19111" -u="rpcuser" -p="rpcpass", I still get the error about missing config fie (true) and not passing dcrwallet RPC credentials in command line options (false)

@ademuanthony That happens because the command line options have not been parsed at this point. Parse the command line arguments first (parser.Parse), before parsing the configuration file.

itswisdomagain · 2018-12-23T15:31:40Z

config.go

+				if err == nil {
+					return loadConfig(appName)
+				}
+				fmt.Println(err)


This should behave as though --init flag was passed, exit after running configure.

Currently, if I get to this point and say yes, it creates the config, prints out The config file has been set successfully but also proceeds to print out the following message which is a little confusing:

available cmds: balance, send, receive, history experimental: send-custom

itswisdomagain · 2018-12-23T15:34:14Z

config.go

+	if err != nil {
+		return inputErr(err)
+	}
+


Can we maintain the pattern below and remove this empty line space?

itswisdomagain · 2018-12-23T15:40:08Z

config.go

+		RpcAddress    string
+		RpcUsername   string
+		RpcPassword   string
+		ServerAddress string


As mentioned in an earlier comment, saving this to dcrcli.conf is unnecessary - the app will not break if this value is not set in config.

There are a couple of other config options that are needed but have default values like RPCCert and NoDaemonTLS. Their absence in dcrcli.conf or in the command-line options won't cause the app to break. Don't see a need to ask the user to supply such information when initialising a conf file.

If we want to ask the user to supply them, then let's make them behave like WalletRPCServer, RPCUser and RPCPassword config options (no default values). They must be provided either in command-line options or saved in dcrcli.conf. if none of the above is done, then the user should be prompted to create a config file or pass those values.

If we're using default values, let's keep using default values until a user on his own goes to set a different value in dcrcli.conf or passes a different value in command-line options

itswisdomagain · 2018-12-23T15:43:57Z

main.go

@@ -42,8 +39,7 @@ func (v *Version) String() string {
 }

 func main() {
-	appName := filepath.Base(os.Args[0])
-	appName = strings.TrimSuffix(appName, filepath.Ext(appName))
+	appName := cli.AppName()

 	config, args, err := loadConfig(appName)


This is the only use of appName in this function. We can do without it. loadConfig is able to access cli.AppName() directly where needed. It currently does so in line 167 of config.go

itswisdomagain · 2018-12-23T16:32:59Z

We'll need to update readme, the part about configuring dcrcli using the sample config file.

barisere

If the configuration directory (~/.dcrcli) does not exist, the program should create it before writing the configuration file. Currently, it does not check whether the directory exists.

$ ./dcrcli --init
Wallet RPC server (default = localhost:19111): 
RPC username (default = rpcuser): 
RPC password (default = rpcpass): error in creating config file: open /home/barisere/.dcrcli/dcrcli.conf: no such file or directory

barisere

Currently, the program does not offer a default when asking the user whether to create the configuration file immediately.

$ ./dcrcli
The config file (/home/barisere/.dcrcli/dcrcli.conf) is missing, and you did not pass dcrwallet RPC credentials in command line options.
Please run 'dcrcli --init' to create the config file or pass RPC credentials by following the instructions set in 'dcrcli -h'
Do you want to create the config file now? (y/n):

The output of the last line of the prompt should be

Do you want to create the config file now? (Y/n):

That indicates that "Y" is the default, so the user can simply hit enter and have the process continue. With the current implementation, the process aborts if the user does not provide an input.
Also check for wrong input. Validate that the input is either "Y" or "N", and it should be case-insensitive.

barisere · 2018-12-24T17:06:00Z

config.go

 			parser.WriteHelp(os.Stderr)
 			return nil, nil, err
 		}
+
+		if preCfg.WalletRPCServer == "" || preCfg.RPCUser == "" || preCfg.RPCPassword == "" {


The absence of those properties does not mean that the configuration file is missing. As @itswisdomagain noted, those properties can be provided via command line options.
I have an idea what the solution may be. First, parse the command line options as I suggested in my previous comment. Then add an else branch to the if block on line 155. The else branch should run only if there was an *os.PathError and the properties you check for on line 161 are empty; i.e., the configuration file is missing, and the associated options were not provided on the command line.

itswisdomagain · 2018-12-24T22:53:33Z

I just came into a shocking revelation. RPC username (config.RPCUser) and RPC password (config.RPCPassword) are currently not used in connecting to dcrwallet daemon. The New function of walletrpcclient.go only uses Wallet RPC server address (config.WalletRPCServer) in connecting to dcrwallet daemon.

The only exception to this is when the user specifies config.NoDaemonTLS to be false and a connection to dcrwallet daemon is attempted over tls. Then RPC server certificate (config.RPCCert) is used together with the server address in establishing a connection to dcrwallet daemon.

This means that the only info we need to run dcrcli effectively is the Wallet RPC server address. If we're creating a config file out of necessity, then the only necessary data to request from the user is Wallet RPC server address. Pretty much makes the whole conf creation process pointless. Why run through so much code just for a single value that we can ask the user to supply via command-line options?

It gets even exciting. The behaviour for dcrwallet is such that if a user does not set a gRPC address to use for incoming connections, localhost:9111 or localhost:19111 is automatically used for mainnet and testnet respectively. If we were to borrow such similar behaviour, we may not even need to worry ourselves whether or not there is a conf file or if the user provided the wallet rpc address via command-line options. If either of those possibilities occur, we use the default address for testnet or mainnet.

The only potential problem with using this default value is - what if the value isn't correct? What if the user set a different dcrwallet listen address? That problem wouldn't really solved by asking the user to specify an address instead of using the default address. The user may still specify an incorrect address. This is a whole other problem that gave birth to issue #77. This PR does not confirm if an rpc server address is valid or not. That's for a different PR in response to #77.

This PR is slowly becoming unnecessary work. I'll create a different PR very soon to remove the unused rpc username and password options from config.go; and also use a default rpc address if none is provided. When that PR gets merged, one is left asking, is there ever a scenario where we'll need to ask a user to create a dcrcli.conf file to continue using the app? Maybe. Maybe not.

My recommendation at this point is, we change this PR's direction. The goal should now be to create a conf file whenever necessary by running dcrcli --init. When a user runs that command, he should be prompted to provide values for all the available dcrcli options, or accept the default values. Then the conf file is created.

barisere · 2018-12-25T08:04:36Z

README.md

 ``` 

-Then edit dcrcli.conf and input your RPC settings. After you are finished, move dcrcli.conf to the `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options
+Follow the command prompt and input your RPC settings. 
+The configuration file, dcrcli.conf will be created in your `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options


What is the default appdata folder on Mac?

Great question. That's one of the changes that the readme needs.
For Mac: ~/Library/Application Support/Dcrcli/dcrcli.conf

Instead of saying

See the output of dcrcli -h for a list of all options"

I think a more relevant information to display here is

To use a different app data folder, use dcrcli --init -C="path/to/desired/app/data/folder"

The Using dcrcli section below should come above this Configuration section. That section (Using dcrcli) should mention using

dcrcli [options] <command> [args...] to run a command

dcrcli -h to view general program help, application options and available commands

dcrcli <command> -h to view help information for a particular command

barisere · 2018-12-25T08:06:22Z

README.md


 ```bash 
-cp sample-dcrcli.conf dcrcli.conf 
+dcrcli --init 


You can prepend this with a "$" to indicate that it's a command to run at the terminal.

$ dcrcli --init

itswisdomagain · 2018-12-25T12:28:38Z

config.go

@@ -39,6 +46,7 @@ type config struct {
 	HTTPServerAddress string `short:"s" long:"serveraddress" description:"Address and port of the HTTP server."`
 	HTTPMode          bool   `long:"http" description:"Run in HTTP mode."`
 	NoDaemonTLS       bool   `long:"nodaemontls" description:"Disable TLS"`
+	Init              bool   `long:"init" description:"Create the config file"`


PR #60 introduces a new flag (--createwallet) for creating wallet (if it doesn't exist already) before using dcrcli. I initially used --init for that use case but decided init was a little ambiguous and not very descriptive.

I want to suggest using a more descriptive flag here, say --createconfig or --config. --config might not work since it could be confused with --configfile used for specifying a config file path.

barisere · 2018-12-25T15:29:41Z

@itswisdomagain --init works, for me. It follows a familiar pattern in CLIs such as git and npm. I would even prefer it as a command.
To create the configuration for running dcrcli:

$ dcrcli init [app_directory]

But that's just me being familiar with other tools that use this style.

itswisdomagain · 2018-12-27T13:30:49Z

@barisere I think in those cases, init is required to use the app, so it makes sense. In our case, init isn't required and does not perform the function of getting the app ready for use. It's just a utility function, as the user can create the conf file without using init

barisere · 2018-12-27T16:10:08Z

@ademuanthony Please address #64 (review).
The configuration directory should be created if it does not already exist.

$ ./dcrcli --init
Wallet RPC server (default = localhost:19111):
RPC username (default = rpcuser):
RPC password (default = rpcpass): error in creating config file: open /home/barisere/.dcrcli/dcrcli.conf: no such file or directory

Also move the error message to its own line.

barisere

When the creation of the configuration file succeeds, the output is as follows.

$ ./dcrcli --init -C ./dcrcli.conf.new
Wallet RPC server (default = localhost:19111):
RPC username (default = rpcuser):
RPC password (default = rpcpass):

The config file has been set successfully

The blank lines are not necessary, and the absolute path to the file written is displayed to the user.
An output like the following looks fine to me. What do you think about it?

$ ./dcrcli --init -C ./dcrcli.conf.new
Wallet RPC server (default = localhost:19111):
RPC username (default = rpcuser):
RPC password (default = rpcpass):
The config file has been written to /home/barisere/code/go/src/github.com/raedahgroup/dcrcli/dcrcli.conf.new

barisere · 2018-12-27T16:45:54Z

I notice we're simply logging errors to the console. I suggest we log them to a file as well, and let the users know about the file. That way, the history of errors does not get lost, and we can get easily introduce telemetry if we want to. The main benefit is being able to go look at detailed error output (including stack traces if necessary) in the log file, and knowing where to start looking for problems. If a user reports an error, we can request the log file for richer context.
@ademuanthony @itswisdomagain

barisere

When the configuration file is created, the default rpccert field should be an actual path on the user's filesystem. Currently it is hard-coded.
Compare

 ; Specify dcrwallet's RPC certificate, or disable TLS for the
 ; rpccert=/home/me/.dcrwallet/rpc.cert

vs

; Specify dcrwallet's RPC certificate, or disable TLS for the
; rpccert=/home/barisere/.dcrwallet/rpc.cert

itswisdomagain · 2018-12-27T20:12:25Z

I notice we're simply logging errors to the console. I suggest we log them to a file as well, and let the users know about the file. That way, the history of errors does not get lost, and we can get easily introduce telemetry if we want to. The main benefit is being able to go look at detailed error output (including stack traces if necessary) in the log file, and knowing where to start looking for problems. If a user reports an error, we can request the log file for richer context.
@ademuanthony @itswisdomagain

Yes! Yes! A new issue should be created for this and addressed in a separate PR. Will need permission from @raedah first though

itswisdomagain · 2018-12-29T01:14:32Z

README.md

 ``` 

-Then edit dcrcli.conf and input your RPC settings. After you are finished, move dcrcli.conf to the `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options
+Follow the command prompt and input your RPC settings. 
+The configuration file, dcrcli.conf will be created in your `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options


Instead of saying

See the output of dcrcli -h for a list of all options"

I think a more relevant information to display here is

To use a different app data folder, use dcrcli --init -C="path/to/desired/app/data/folder"

itswisdomagain · 2018-12-29T01:18:38Z

README.md

 ``` 

-Then edit dcrcli.conf and input your RPC settings. After you are finished, move dcrcli.conf to the `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options
+Follow the command prompt and input your RPC settings. 
+The configuration file, dcrcli.conf will be created in your `appdata` folder (default is `~/.dcrcli` on Linux, `%localappdata%\Dcrcli` on Windows). See the output of `dcrcli -h` for a list of all options


The Using dcrcli section below should come above this Configuration section. That section (Using dcrcli) should mention using

dcrcli [options] <command> [args...] to run a command

dcrcli -h to view general program help, application options and available commands

dcrcli <command> -h to view help information for a particular command

itswisdomagain · 2018-12-29T01:38:51Z

config/config.go

+	defaultRpcAddress  = "localhost:19111"
+	defaultRpcUsername = "rpcuser"
+	defaultRpcPassword = "rpcpass"
+	ServerAddress      = "127.0.0.1:7778"


This isn't relevant, we already have defaultHTTPServerAddress

itswisdomagain · 2018-12-29T02:09:30Z

config/config.go

@@ -36,6 +44,7 @@ type Config struct {
 	HTTPServerAddress string `short:"s" long:"serveraddress" description:"Address and port of the HTTP server."`
 	HTTPMode          bool   `long:"http" description:"Run in HTTP mode."`
 	NoDaemonTLS       bool   `long:"nodaemontls" description:"Disable TLS"`
+	Init              bool   `long:"init" description:"Create the config file"`


In line with this comment and this reply, I want to re-recommend this flag be renamed.

It's okay if git and npm uses init, you can't use those programs without first issuing that command, "init" makes sense.

In our case, "init" isn't a pre-requirement to use the program, so it's quite misleading... What this does isn't setting up the program for use, but instead creating a config file on-demand - at any time the user says he wants to create one. I think --config is appropriate

itswisdomagain · 2018-12-29T02:12:23Z

config/config.go

+		return fmt.Errorf("error receiving input: %s", err.Error())
+	}
+
+	var rpcAddress, rpcUsername, rpcPassword string


The rpcUsername and rpcPassword values are not used in the dcrcli program. They should be completely removed from the list of config options and this function should not request values for them from the user.

Since those 2 options are no longer required, it doesn't make sense for a user to run dcrcli --init or dcrcli --config just to set rpcAddress. I think it'd be appropriate to prompt user to provide values for all or most of the application options. One exception would be the ConfigFile option (only makes sense in command-line options).

Then use the values provided to create the config file.

should we remove the config flag entirely and just request the address from the user at the point of loading the config options in the program?

Remove the config/init flag? No. We've done too much work here already to throw it away.

We also should not request the address from the user. To connect to dcrwallet, the dcrwallet cert file path is also needed but we don't request that from the user either. If the user doesn't set a value in config or in command-line options, we use the default. That's currently what we're doing for the wallet rpc server address as well.

itswisdomagain · 2018-12-29T02:30:46Z

config/config.go

+		ServerAddress string
+	}{
+		RpcAddress: rpcAddress, RpcUsername: rpcUsername, RpcPassword: rpcPassword,
+		ServerAddress: ServerAddress,


We should not directly save a default/static value to config file. Before doing so, the user should be prompted for a value to use with the option of retaining the default value

itswisdomagain · 2018-12-29T02:32:37Z

config/dcrcli.config.go


 ; Turn HTTP mode on (http=true or http=1) or off (http=false or http=0).
 ; HTTP mode is off by default.
-; http=true
+; http=true`
+


We need to remember to update this file as more application options/flags are added. For example, we now have UseTestNet (; testnet=1)

If the config file is not found and the user did not pass Dcrwallet credentials in command line options, propose to guide the user in creating a new config file and do same if the user accepts, else quit the application use text/template for the conf parsing Clean up Default value for RPC credentails Suggest and use default value for the RPC credentials Refactored the code to use the already existing RequestInput Modified the terminalpromt.RequestInput func to eccept nil validation func Cleaning up remove empty validator define default values as constants configure handler moved the config setup to a handler and inform the user of the appropriate configuration command at startup if config not found. removed the sample-dcrcli.conf to leave the template string in the handler as the single source of truth for the config file Requested changes Renamed 'configure' command to 'config'. removed appName from cli struct and use AppName fn in its place. detached invalidCommandReceived from cli struct. removed unnecessary create config dialog. point out the default values. Created a config flag and call the call configure Moved the configure func to config.go and also call the func on start if the config file is not found Created a config flag and call the call configure Moved the configure func to config.go and also call the func on start if the config file is not found Merge remote-tracking branch 'origin/iss-50-conf-detection' into iss-50-conf-detection configuration instruction in readMe

itswisdomagain · 2018-12-30T20:38:26Z

I'm gonna close this PR @ademuanthony and @barisere. The issue this PR took to solve is outdated and irrelevant. But the work done so far, isn't completely unusable. We'll channel it into a more relevant issue.

I'll create the new issue and reference this PR. However, this PR should not be re-opened for the new issue. A new PR should be created. Most of the code changes here can and should be retained though.

I've updated the issue being fixed by this PR (#50) with a reason for closing both the issue and this PR. You can find the reason here planetdecred/godcr#50 (comment)

ademuanthony changed the title ~~WIP - detect if dcrcli.conf is not configured~~ detect if dcrcli.conf is not configured Dec 17, 2018

raedah added this to In Progress in godcr board Dec 17, 2018

raedah moved this from In Progress to For Review in godcr board Dec 17, 2018

barisere requested review from barisere and itswisdomagain December 17, 2018 15:31

barisere added the enhancement New feature or request label Dec 17, 2018

barisere suggested changes Dec 17, 2018

View reviewed changes

itswisdomagain suggested changes Dec 18, 2018

View reviewed changes

config.go Outdated Show resolved Hide resolved

config.go Outdated Show resolved Hide resolved

itswisdomagain suggested changes Dec 18, 2018

View reviewed changes

cli/terminalprompt/terminalprompt.go Outdated Show resolved Hide resolved

itswisdomagain mentioned this pull request Dec 22, 2018

[WIP] ./dcrcli help #63

Closed

itswisdomagain suggested changes Dec 22, 2018

View reviewed changes

itswisdomagain mentioned this pull request Dec 23, 2018

detect if dcrcli.conf is not configured #50

Closed

itswisdomagain suggested changes Dec 23, 2018

View reviewed changes

itswisdomagain mentioned this pull request Dec 23, 2018

dcrwallet rpc connection error #77

Closed

barisere suggested changes Dec 24, 2018

View reviewed changes

barisere reviewed Dec 24, 2018

View reviewed changes

barisere suggested changes Dec 24, 2018

View reviewed changes

barisere reviewed Dec 25, 2018

View reviewed changes

itswisdomagain changed the title ~~detect if dcrcli.conf is not configured~~ [WIP] detect if dcrcli.conf is not configured Dec 25, 2018

itswisdomagain moved this from For Review to In Progress in godcr board Dec 25, 2018

itswisdomagain suggested changes Dec 25, 2018

View reviewed changes

ademuanthony changed the title ~~[WIP] detect if dcrcli.conf is not configured~~ detect if dcrcli.conf is not configured Dec 27, 2018

barisere suggested changes Dec 27, 2018

View reviewed changes

itswisdomagain mentioned this pull request Dec 27, 2018

store dcrcli logs in log file #84

Closed

itswisdomagain moved this from In Progress to For Review in godcr board Dec 28, 2018

itswisdomagain suggested changes Dec 29, 2018

View reviewed changes

itswisdomagain mentioned this pull request Dec 29, 2018

update readme #74

Merged

barisere mentioned this pull request Dec 30, 2018

Make packages commands and config disjoint #86

Merged

ademuanthony added 2 commits December 30, 2018 14:56

config file is optional

51c44d8

itswisdomagain closed this Dec 30, 2018

itswisdomagain removed this from For Review in godcr board Dec 30, 2018

itswisdomagain mentioned this pull request Jan 1, 2019

add cli command to create config file #107

Closed

detect if dcrcli.conf is not configured #64

detect if dcrcli.conf is not configured #64

Conversation

ademuanthony commented Dec 17, 2018 • edited Loading

barisere left a comment

Choose a reason for hiding this comment

ademuanthony commented Dec 21, 2018

itswisdomagain left a comment

Choose a reason for hiding this comment

ademuanthony commented Dec 22, 2018 • edited Loading

itswisdomagain left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itswisdomagain commented Dec 23, 2018

barisere left a comment

Choose a reason for hiding this comment

barisere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itswisdomagain commented Dec 24, 2018

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

barisere commented Dec 25, 2018

itswisdomagain commented Dec 27, 2018

barisere commented Dec 27, 2018

barisere left a comment • edited Loading

Choose a reason for hiding this comment

barisere commented Dec 27, 2018 • edited Loading

barisere left a comment

Choose a reason for hiding this comment

itswisdomagain commented Dec 27, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itswisdomagain commented Dec 30, 2018 • edited Loading

ademuanthony commented Dec 17, 2018 •

edited

Loading

ademuanthony commented Dec 22, 2018 •

edited

Loading

barisere left a comment •

edited

Loading

barisere commented Dec 27, 2018 •

edited

Loading

itswisdomagain commented Dec 30, 2018 •

edited

Loading