Question about GSOC pwctl CLI project

[qazxcdswe123]

Hi, I'm interested in the GSOC pwctl CLI project, and I have some questions here

1. Is the CLI targeting pgwatch3 (or pgwatch2)?
2. It seems that only API below are implemented, if the CLI is going to interact with pgwatch via REST API, does that mean these are all functionalities I need to implement?

type apiHandler interface {
	GetDatabases() (string, error)
	AddDatabase(params []byte) error
	DeleteDatabase(id string) error
	UpdateDatabase(id string, params []byte) error
	GetMetrics() (res string, err error)
	AddMetric(params []byte) error
	DeleteMetric(id int) error
	UpdateMetric(id int, params []byte) error
	GetPresets() (res string, err error)
	AddPreset(params []byte) error
	DeletePreset(name string) error
	UpdatePreset(id string, params []byte) error
	GetStats() string
	TryConnectToDB(params []byte) error
}

Thanks

[pashagolub]

Hi, thanks for your interest!

Yes, for the GSoC scope these only are fine. Yes, pwctl is targeting v3, v2 is in maintenance mode.

[qazxcdswe123]

I've done some research, here are my plan

In theory, all the interactions should be done via REST API

• pwctl -U admin -P admin -h localhost:8080 subcommand
  • -U: username (or -u, capital U to be consistent with psql)
  • -P: password (should we pass it via command line arguments or enter interactively?) (or -p?)
  • -h: host (optional, default to localhost:8080)

1. get (by id)
   • database
   • metric
   • preset
   • stats
2. add: probably too many parameters? Maybe use yaml file if needed.
   • database
   • metric
   • preset
3. delete (by id)
   • database
   • metric
   • preset
4. update: probably too many parameters? Maybe use yaml file if needed.
   • database
   • metric
   • preset
5. status: check status by ???
6. sink: need to add additional api for this (if we are going to do it via REST API)
7. pause: need to add additional api for this

Each of these top-level commands might have additional flags or arguments. For example:

• get databases would not require any additional arguments.
• delete database --id "db123" would require the ID of the database to delete.
• update metric --id 42 --params '{"md_name":"test"}' would require both the ID of the metric and the params to update. But how are the user supposed to know the param key?

---

Questions:

1. Is it possible to add id to the database schema since currently there are no id field (take monitored_db for example)
2. authentication is questionable
3. add and update is questionable
4. is this kind of thing what you want?

[pashagolub]

Is it possible to add id to the database schema since currently there are no id field (take monitored_db for example)

Yes, it is possible. But I prefer to use human readable name for monitored sources. And it is a responsibility of a backend to answer properly. Database schema is not covered by the scope of this project.

authentication is questionable

We can use the same as for WebUI. But for the start we can omit auth and get back to it when the whole functionality is stable.

add and update is questionable

if we will add those, then probably we will use .yaml files as input parameters. We can skip them for the start

is this kind of thing what you want?

yes, that looks pretty much the same in my head. I'm OK with discussing and adopting the best approaches.

Thanks for your work!

[pranayjoshi]

Hi, I had a bit different approach.
By reading the project description and expected outcomes, I think the most important thing is that pwctl should be easy to use.
Therefore I would try to keep the commands as concise and readable as possible.

Therefore, for authentication purpose, I was planning to keep it something similar to github, where a user can login once
e.g. pwctl login -U admin -P admin -h localhost:8080
make it a one time thing and then continue with the commands.

pwctl status
pwctl dropdb <id>
many more.

[pranayjoshi]

@pashagolub from
Description: Pause the monitoring activities of the pgwatch instance running on localhost at port 8080. are you referring to the
EmergencyPauseTriggerfile() function?

[pashagolub]

The implementation is up to me. The scope of this project is to ensure the command.

[qazxcdswe123]

Just recovered from COVID, I had my proof of concept implementation on https://github.com/qazxcdswe123/pwctl, which currently supports pwctl list database

I've also refined some parameters.

• For get database, we can pass --id or --name to get details, by default we should use list and it only shows id, name and connection string
• same for get metrics, but maybe we should make it interactive if invoked directly since there are too many of them (110 currently). It should also behave normally when piped with grep. Not sure though.
• Specify id or name when using delete
• For status, maybe we can add a dedicated /status endpoint? Or find keyword in webui. I've noticed the current PR that uses docker ps to check the status, but it may not be running on the same machine and container name can varies

[pashagolub]

Looks nice. Need to check that

[qazxcdswe123]

I've restructured the code to make it more clear and extensible, this is probably my final design. What do you think?
Preview of the list command

[Xunop]

Looks nice. Need to check that

Hi, I've organized the pwatch3 API and uploaded it here. I also created a simple demo to help me understand the idea of pwctl.

The pwatch3 API is very simple and easy to use. Interacting with the pwatch3 API is not difficult, but the challenging part is designing the commands and flags to make it user-friendly.

Some APIs use CLI for control, which can be complex(such as the add operation). I think they need to be simplified (perhaps by adding more flags) to make them easier to use instead of using JSON strings to interact with pwctl. Additionally, the output of pwctl is important. I believe it needs to consider both the output format and content. The storage of the token is also need to be considered. I don't know if it is a good idea to store the token in a file.

I would appreciate any suggestions or comments from developers. Your insights would be valuable for further discussions.

[pashagolub]

Thanks! Will check! Command with complex input should accept yaml, I believe. Will get there.

[pranayjoshi]

Hi @pashagolub , https://github.com/cybertec-postgresql/pgwatch3/blob/master/src/sinks/json_file.go do we have to mimic the behavior of this API for the sink task for the csv file? Also, if yes then what data are we required to store? Is it the same metrics data similar to the sinks package?

[pashagolub]

We don't yet have csv sink. Not sure we need it at the moment, because every metric will have it's own file and it quickly may become cumbersome

[pranayjoshi]

But the Json sink serves the same purpose right?

[pashagolub]

Would you please rephrase the initial question. Seems I'm lost completely.

[pranayjoshi]

In short, does the json_file.go performs the same sink operation(for a json file instead of csv file) that is asked in the pwctl prompt Add a file sink to the pgwatch instance running on localhost at port 8080, directing the output to a CSV file named "output.csv". ?

[krisavi]

Just as a remark, the CLI should be kind of read only in case you don't use webUI and configuration DB to manage pgwatch, but use config file based setup. As reconfiguring anyways is done via configuration file, then the pwctl could only be used for debugging purpose.

[pashagolub]

That won't be true in a week, I believe. There is pull request in WIP allowing to change file based configs as well.

But anyway CLI should use REST API pgwatch3 provides.

[krisavi]

I would prefer to have possibility to lock config file from being edited except config management tools.

Probably could be done with unix permissions granting read right for pgwatch group, but file ownership to config management tool user.

Just hope that it would give some sensible error when trying to change file that may not be changed by pgwatch.

[pashagolub]

To disallow config file change one can disable REST API