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

Add: configure necessary plugins in the config and install them automatically #3041

Closed
wants to merge 15 commits into from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Dec 29, 2023

This pr adds necessary plugins to the config such that those plugins will be installed automatically.

Jim8y and others added 4 commits December 29, 2023 22:05
@@ -20,7 +20,8 @@
"Path": "",
"Password": "",
"IsActive": false
}
},
"Plugins": ["LevelDBStore"]
Copy link
Member

Choose a reason for hiding this comment

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

If we define the plugins in the config, we should not try to find them in the folder

Copy link
Contributor Author

@Jim8y Jim8y Dec 30, 2023

Choose a reason for hiding this comment

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

what? Its not we defined them in the config, but allowing developers to have them in the config. And i dont understand your we should not try to find them in the folder.

Copy link
Member

Choose a reason for hiding this comment

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

And i dont understand your we should not try to find them in the folder.

Currently the plugins are "installed" if they are present in the folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly, the install command will check if they exists, if so, the install command will return directly.

Copy link
Member

Choose a reason for hiding this comment

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

People may get confused when they see this. Because we don't update Plugins list?

@cschuchardt88
Copy link
Member

cschuchardt88 commented Jan 14, 2024

Maybe add a check for the Storage JSON node in the config file than check if the directory exists. If not than default to MemoryStore. This way it allows them to download whatever storage they want. RocksDb or Leveldb. Also add AutoStart in the config file for starting the node's syncing process or not in MemoryStore Mode. Like a MemoryStore section in the JSON config file. This way they can just download what they want for plugins.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 15, 2024

some plugins do not have cknfig.json.

@cschuchardt88
Copy link
Member

some plugins do not have cknfig.json.

I have never seen this. Maybe it been removed.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 15, 2024

some plugins do not have cknfig.json.

I have never seen this. Maybe it been removed.

config.json is not required for all plugin, so.

@cschuchardt88
Copy link
Member

Didn't realize was a typo.

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 15, 2024

Didn't realize was a typo.

LMAO

@cschuchardt88
Copy link
Member

cschuchardt88 commented Jan 15, 2024

PR #3092 uses the Plugins section in the json config file. Can you add a json property under it?

Like this :

    "Plugins": {
      "DownloadUrl": "https://api.github.com/repos/neo-project/neo-modules/releases",
      "Install": []
    }

@Jim8y
Copy link
Contributor Author

Jim8y commented Jan 15, 2024

PR #3092 uses the Plugins section in the json config file. Can you add a json property under it?

Like this :

    "Plugins": {
      "DownloadUrl": "https://api.github.com/repos/neo-project/neo-modules/releases",
      "Install": []
    }

I saw you are using it, will update after it is merged.

@cschuchardt88
Copy link
Member

I think this kind of silly. It should be in MemoryStore storage 1st time download and not start the NeoSystem node. Than the user can pick if they want rocksdb or leveldb. Anyone trying to run a node will have someone background of configuration knowledge.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 5, 2024

whatever the logic is, we dont change it for now. This is not a personal project, being used by exchages, it has being this way , it will stay this way until a new big version.

@cschuchardt88
Copy link
Member

whatever the logic is, we dont change it for now. This is not a personal project, being used by exchages, it has being this way , it will stay this way until a new big version.

I understand.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 5, 2024

@shargon @NGDAdmin

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

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

Honestly, I don't really like having two places to define the list of plugins, I don't really understand what problem it solves, if we release with the plugin already in the correct directory, it only complicates the wheel

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 6, 2024

It enables developers or users to specify their own config with plugins of their own choice. Without these, they have to check them one by one and install them manually one by one

@shargon
Copy link
Member

shargon commented Feb 6, 2024

It enables developers or users to specify their own config with plugins of their own choice. Without these, they have to check them one by one and install them manually one by one

I prefer to use arguments like --install X,Y,Z and they can run the node with this argument

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 7, 2024

I prefer to use arguments like --install X,Y,Z and they can run the node with this argument

exactly, but this also means they have to remember every plugins, type in every time they use it. say user want 5 plugins, 10 plugins.

@cschuchardt88
Copy link
Member

A sample scripting protocol would be good here. Would work like a bash script. This scripting system would allow a user to run neo-cli within the cli. This is the best solution. Aka neo-cli batch file.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 7, 2024

I am working to add scripts, but waiting for shargon to merge that install plugsin scirpt pr.

@cschuchardt88
Copy link
Member

@Jim8y Let me clear this up. neo-express has *.batch files that execute the commands for the cli in the cli. That is what i am proposing. To be able to execute cli commands in neo-cli as if you were to type to them. For example install LevelDBStore, show state, and etc.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 7, 2024

@Jim8y Let me clear this up. neo-express has *.batch files that execute the commands for the cli in the cli. That is what i am proposing. To be able to execute cli commands in neo-cli as if you were to type to them. For example install LevelDBStore, show state, and etc.

So?

@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 7, 2024

No configuration required. System admins can pick and choose their own plugins to install. In addition, execute other commands. Let's say when they open the cli they want show state display, up all the time. Wouldn't it be better to type it once in a script file? Instead of everytime you open neo-cli and have to wait for the prompt to load.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 8, 2024

But we have a configuration system, it has being the way how things work in neo, you cant just say its not good and we build another system, people are using it (though not many), historical burdens you must carry and respect. And having it in the configuration does not mean you can not have it supported from script.

@cschuchardt88
Copy link
Member

But we have a configuration system, it has being the way how things work in neo, you cant just say its not good and we build another system, people are using it (though not many), historical burdens you must carry and respect. And having it in the configuration does not mean you can not have it supported from script.

Ok. Doesn't mean we can't change for the better. Also I am not saying remove it. More than one option is good. Because if one breaks or is limited the other can help. If your OS did this and it broke on an update. How would you fix it or get it running?

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 8, 2024

But we have a configuration system, it has being the way how things work in neo, you cant just say its not good and we build another system, people are using it (though not many), historical burdens you must carry and respect. And having it in the configuration does not mean you can not have it supported from script.

Ok. Doesn't mean we can't change for the better. Also I am not saying remove it. More than one option is good. Because if one breaks or is limited the other can help. If your OS did this and it broke on an update. How would you fix it or get it running?

This is the issue that we have being facing since the begining. User may customize their config, but when we update the node, we sometimes also update the config, making the user has to update their own config..... samething.

@cschuchardt88
Copy link
Member

But we have a configuration system, it has being the way how things work in neo, you cant just say its not good and we build another system, people are using it (though not many), historical burdens you must carry and respect. And having it in the configuration does not mean you can not have it supported from script.

Ok. Doesn't mean we can't change for the better. Also I am not saying remove it. More than one option is good. Because if one breaks or is limited the other can help. If your OS did this and it broke on an update. How would you fix it or get it running?

This is the issue that we have being facing since the begining. User may customize their config, but when we update the node, we sometimes also update the config, making the user has to update their own config..... samething.

System like this you wouldn't need to update the configuration. A language based script can't change.

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 8, 2024

System like this you wouldn't need to update the configuration. A language based script can't change.

You dont get my point,,,, its not about good or bad,,,, its that the system works like this, that's it,,,people goes to config to configure the node, even with your script, they still need to configure the config file.

@cschuchardt88
Copy link
Member

No i understand completely. But I'm just saying. But at what point does a configuration file become too much to manage?

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 9, 2024

No i understand completely. But I'm just saying. But at what point does a configuration file become too much to manage?

Too much to manege in one place or go multiple places to manage different things? Which one do you prefer?

@shargon
Copy link
Member

shargon commented Feb 9, 2024

No i understand completely. But I'm just saying. But at what point does a configuration file become too much to manage?

Too much to manege in one place or go multiple places to manage different things? Which one do you prefer?

This question is too generic, could you give me one example of a tool where the plugins are installed by the config file?

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 9, 2024

This question is too generic, could you give me one example of a tool where the plugins are installed by the config file?

Why would I give you one example of such thing? Making it easy to use is the general purpose. Neo is not hard to use enough? how many users do we still left to lose? Do you know that one more step you add, 50% users you lose. Do you know what it means to install plugins via scripts? It means every time they want to reinstall their node, or they want to update neo core, they must 1. update the new config, 2. then run the script to install plugins. Since they already need to update the config, why dont we just allow them to install from the config? done it in one step? Should we just film them a video showing them, Ok, when you update the node, please do this first, then do that, then to other things? What is the point? Oh, wait a second, its not just the neo-cli config, its almost every plugins they installed config. manully update all of them (worst case). Dont you think it is already complex enough?

@Jim8y
Copy link
Contributor Author

Jim8y commented Feb 9, 2024

This will not be added.

@Jim8y Jim8y closed this Feb 9, 2024
@Jim8y Jim8y deleted the plugin-config branch February 9, 2024 11:07
@neo-project neo-project deleted a comment from Jim8y Feb 9, 2024
@cschuchardt88
Copy link
Member

cschuchardt88 commented Feb 9, 2024

This question is too generic, could you give me one example of a tool where the plugins are installed by the config file?

Why would I give you one example of such thing? Making it easy to use is the general purpose. Neo is not hard to use enough? how many users do we still left to lose? Do you know that one more step you add, 50% users you lose. Do you know what it means to install plugins via scripts? It means every time they want to reinstall their node, or they want to update neo core, they must 1. update the new config, 2. then run the script to install plugins. Since they already need to update the config, why dont we just allow them to install from the config? done it in one step? Should we just film them a video showing them, Ok, when you update the node, please do this first, then do that, then to other things? What is the point? Oh, wait a second, its not just the neo-cli config, its almost every plugins they installed config. manully update all of them (worst case). Dont you think it is already complex enough?

Either way you look at it. You need to do two steps.

  1. Edit a file
  2. Write something

It's just one can break everything and the other one can't. Once you have the script. The script will never change. Unless you want to add more plugins. For a config file it is always changing for each and every system and environment. Script files wouldn't change in different environments or systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants