Spooltracker#1379
Conversation
|
From a security standpoint, your plugin looks fine to me. I did notice a few oddities:
However, I'll leave comments regarding code quality, logic, and functionality to the other reviewers. |
I appreciate the help. I don't have much experience with coding. As im sure you have seen i made this whole plugin with ai so im glad someone who actually knows what they are doing is looking into it.
|
|
Let me know here once those updates to setup.py have been completed @thatguymendel, notice the plugin_url is also not right with your github username in it. |
|
You could simplify the API permissions check by moving it to the top of your on_api_command definition before the if/elif checks and remove it from each if check, that way you only have one since they are all using the same permission... |
|
@jneilliii I added the info into setup.py |
|
I think you may have misunderstood what I was saying. Instead of having the check underneath each if/elif, just have the one at the beginning of the function. The actual way you have implemented is technically fine, but more of a best practice kind of thing. The idea being exit (in your case the flask abort) as early as possible to reduce processing time. If the check is at the beginning of the function then you don't have to do any further if/else logic processing if it fails. I know with today's high-end compute it's probably not perceivable to the end-user, but in certain cases it could be. I'd highly suggest if you're not going to use the opportunity to learn from seasoned developers like us, and instead continue the path of vibe coding, at least look at how other plugins have implemented these things. I guarantee that at least one of my 50+ plugins have an example of almost anything you are attempting to do. Just doing that for custom permissions could have probably been completed in at most 1/2 hour of looking and copying/pasting from another plugin. Here's the changes for a custom permissions I just recently did for a plugin as reference. The relevant parts are the import statement at top, the |
|
I need to mention that if you put the plugin on the official OctoPrint plugin repository, it is also expected that you are able to maintain it. People will ask for additional features, will probably find bugs, etc and you'll need to take care of those, plus you should also follow OctoPrint's development in case something on your plugin needs a change down the road to stay compatible. You should be clear that maintaining will become your responsibility after publishing. If you only created the plugin for your own workflow and are not open or able to maintain it longer term, putting it on the repository sadly doesn't make a lot of sense. |
|
I created the plugin primarily for my own workflow, but finding and fixing bugs is still in my interest since I’m using it myself. I didn't really think about maintaining it long term. What does it usually take to maintain a plugin? The main reason I wanted to publish this plugin was to show other people that you can now make an entire OctoPrint plugin with little to no coding experience using AI. This whole plugin was an experiment to see how much I could do without having to really dive into coding or figure out the OctoPrint plugin system myself. I didn’t write a single line of code to make this plugin, and I think the fact that it works is incredible. |
|
Here are my two cents on the matter — please keep in mind this is just my personal opinion and may not reflect the project's official stance. I definitely don’t want to discourage anyone from creating and sharing plugins — in fact, I believe that’s beneficial for both the project and the community, so I’m also a bit sorry for what I’m about to say. I'm not sure if adding to the official repository a plugin that was entirely "vibe-coded" by a maintainer who doesn’t actually know how to program aligns with the current contribution guidelines:
The same applies to the concept behind your plugin:
That said, I’d like to gently remind that maintaining a plugin requires ongoing long-term effort (monitoring updates and future breaking changes in OctoPrint, handling bug reports, etc.) as well as responsibility—such as trying to avoid causing printer failures or, even worse, fires. Plugins interact directly with OctoPrint’s core and can significantly affect its operation, performance, safety, and security. There are responsibilities both toward OctoPrint as an open source project and to the users who rely on this software to run their printers safely. In my opinion, those responsibilities cannot be fully entrusted to AI or to developers who don’t truly understand the code they publish. |
|
Just to add my two cents as well, as the creator & maintainer of OctoPrint (and the person who gets the calls from reporters when the ecosystem ends up in the news for some reason):
That is definitely not the message that we want to see sent however and I'm currently working on some clearer guidelines in that regard. We expect the maintainers of the plugins that are published on the official Plugin Repository and thus easily accessible for everyone using OctoPrint to at least be able to maintain their plugins even if their favourite genAI goes down. That means a minimum amount of coding capability, development experience and understanding of why their plugin works. Also, a commitment to keep things updated and secure. Sadly, a growing list of abandoned plugins is already causing grief in the community, and the last thing we need is more plugins added to that. You are always welcome to create your plugin, publish it on GitHub or some other forge and make it available to others that way. But if you want to publish it on the plugin repository, we expect some more responsibility than vibe coding can offer. |
|
It's certainly incredible what AI can do these days - and I'm always impressed by how much it can help in day-to-day workflows. I'd admit that I don't use it enough in my current job and I'm always impressed when I can get things done faster with AI. That said, I think I'd agree with the sentiments shared - it's great to write your own plugin and I'd hate for anyone to be discouraged from doing so. But putting it on the repository means that any users may submit anything to you, which you have to decide & act upon - are they bugs, feature requests or just limitations out of scope of your plugin. If you don't have the experience or background for this, that'll be a tough process and you'll just have a bad time, and so will the users. I'd recommend perhaps sharing your experience on the community forums - your experience is still valuable. Let people know, that if their workflow isn't quite possible with the existing offering they could do something about this with the use of AI. I think it's very powerful and good to point out the use case. |
i added more information on the plugins page and i also implemented the security changes into the plugin that @jacopotediosi recommended