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

Fix/privacyjson #1379

Merged
merged 12 commits into from
Oct 7, 2022
Merged

Fix/privacyjson #1379

merged 12 commits into from
Oct 7, 2022

Conversation

ersonp
Copy link
Contributor

@ersonp ersonp commented Oct 5, 2022

Did you run make format && make check?

Fixes #1361

Changes:

  • Fixed config priv and visor priv

How to test this PR:

  1. Run ./skywire-cli config priv get
  2. Run ./skywire-cli config priv get --json
  3. Run ./skywire-cli config priv set
  4. Run ./skywire-cli config priv set --json
  5. Start Visor
  6. Run ./skywire-cli visor priv get
  7. Run ./skywire-cli visor priv get --json
  8. Run ./skywire-cli visor priv set
  9. Run ./skywire-cli visor priv set --json
  10. On Postman GET http://localhost:8000/api/visors/<pk>/privacy
  11. On Postman PUT http://localhost:8000/api/visors/<pk>/privacy with body
{
    "display_node_ip": true,
    "reward_address": ""2jBbGxZRGoQG1mqhPBnXnLTxK6oxsTf8os6""
}

ersonp added 10 commits October 5, 2022 20:23
This commit add a new pkg called privacyconfig.

The code from cli config private is moved in this pkg and is then also used by the API's SetPrivacy and GetPrivacy.

We fix the API by removing the script code and use funcs privacyconfig.SetReward and privacyconfig.GetReward.
This commit changes the lowercase vairables to use cammel case instead to keep the consistency of the codebase following the basic Naming Rules and Conventions of golang.
This commit changes how the structs are initilized as per the guidleines used in the skywire project.
This commit removes the param pathStr from SetReward because it is no longer needed.
Update config priv subcommands set and get to use flags output, pkg and user that are used by main config instead of using it's own set of flags in order to have the functionality be the same.
@ersonp ersonp marked this pull request as ready for review October 7, 2022 14:20
@ersonp
Copy link
Contributor Author

ersonp commented Oct 7, 2022

skywire-cli config priv get

read reward address & privacy setting from file

Usage:
  skywire-cli config priv get [flags]

Flags:
  -o, --out string   read privacy config from: ./local/privacy.json
  -p, --pkg          use path for package: /opt/skywire/local
  -u, --user         use paths for user space: /home/erson/.skywire/local

Global Flags:
      --json         print output in json
      --rpc string   RPC server address (default "localhost:3435")

Output flags now behave like the output flags in config gen

@0pcom 0pcom self-requested a review October 7, 2022 16:20
Copy link
Collaborator

@0pcom 0pcom left a comment

Choose a reason for hiding this comment

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

I've noticed a lot of improvements and streamlining of the cli command code as compared to my initial work. Very good.

One thing I will just note ; on the flags for skywire-cli config priv

we are again in a situation where, no matter what permissions you have, you shouldn't use one of the two flags (-p or -u). Either you won't be able to write the file if you use the -p flag without root or you will create a root-owned file in the userspace using the -u flag as root

for skywire-cli config gen those flags do more than just set the output path, they set paths in the config used by the visor. Additionally, the output path of the skywire config can be overridden with the -o flag, by design.

Since we don't have such considerations in this instance, would it not be better either as I had it before, or with the config being written to the correct path for the current permissions?

The package is basically the point where we can facilitate running skywire at the user level, and it does not exactly support this currently. So it's actually just extra complexity in the config priv subcommand.

most people will use either the hypervisor UI to set this file or the visor. At the point it's done from the visor, the current local path is used, which the visor in theory has permissions for.

I am going to use the skywire-cli config priv subcommand. I may make a script that will use it too, so I would like the simple and intuitive implementation that writes to the only local path that is really used currently in the packages.

@0pcom 0pcom merged commit ce085b8 into skycoin:develop Oct 7, 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.

2 participants