Skip to content

Add pinchflat installation script for testing #693

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AkuchiS
Copy link

@AkuchiS AkuchiS commented Jul 9, 2025

This pull request adds the pinchflat.sh installation script to the ProxmoxVE repository.

Overview:
The pinchflat.sh script automates the installation and setup process for the pinchflat application, which is a YouTube downloader. This script simplifies the installation by handling dependencies, setting up a Python virtual environment, and configuring a systemd service for easy management.

Key Features:
Installs necessary dependencies for the pinchflat application.
Sets up a Python virtual environment using pipenv.
Configures a systemd service to manage the application as a background service.
Backs up any existing installations before performing the update.

Source:
This script is based on the original work found in the kieraneglin/pinchflat repository.

Usage:
Once the script is executed, users can access the pinchflat application via the URL specified in the output, typically at http://:8945.

@AkuchiS AkuchiS requested a review from a team as a code owner July 9, 2025 02:36
@AkuchiS
Copy link
Author

AkuchiS commented Jul 9, 2025

Hi there,

I noticed that the auto-labeling action for my pull request has failed due to permission issues. The error message indicates that it does not have the necessary permissions to create labels on this repository.

Here's the relevant part of the error message:

"You do not have permission to create labels on this repository."

Could you please check the permissions for the auto-labeler action? I want to ensure that my script can be reviewed and potentially merged.

Also, I need to change the port from 8081 to 8945 within the script, at the bottom. I used the former for testing to minimise conflicts however, Keiran utilises the later in his script.

Thank you for your help!

Copy link
Member

@CrazyWolf13 CrazyWolf13 left a comment

Choose a reason for hiding this comment

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

I think there is a more general problem

We have 3 different files:

  • Frontend (json)
  • Update script (ct/pinchflat.sh)
  • install script (install/pinchflat-install.sh)

You seemd to have only the update script but made it in install dir without -install subfix, please correct this and add the other files:
here our guide: https://github.com/community-scripts/ProxmoxVE/wiki/CONTRIBUTING

@AkuchiS AkuchiS requested a review from a team as a code owner July 9, 2025 07:58
@AkuchiS
Copy link
Author

AkuchiS commented Jul 9, 2025

Thank you for the review! I've made the following changes based on your feedback:

  • Updated the source line to point to the ProxmoxVED repository.
  • Changed the copyright and author information.
  • Adjusted the capitalization in the message for backing up.
  • Replaced git clone with fetch_and_deploy as recommended.
  • Removed the migration logic since this is a new script and does not require it.

✅ Moved pinchflat.sh into ct/
✅ Created pinchflat-install.sh in install/
✅ Added pinchflat.json in frontend/

Please let me know if there are any further changes needed!

@@ -0,0 +1,115 @@
#!/usr/bin/env bash
source <(curl -fsSL https://raw.githubusercontent.com/community-scripts/ProxmoxVED/main/misc/build.func)
# Copyright (c) 2021-2025 ProxmoxVED
Copy link
Member

Choose a reason for hiding this comment

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

change this according to our other scripts.

msg_ok "Backup created"

msg_info "Fetching and deploying latest ${APP} release"
fetch_and_deploy https://github.com/kieraneglin/pinchflat
Copy link
Member

Choose a reason for hiding this comment

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

that's not how this function works, please read through (or tell your AI Model, which you used) the description of the function I the first time.

[Install]
WantedBy=multi-user.target
EOF
msg_ok "Patched systemd Service"
Copy link
Member

Choose a reason for hiding this comment

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

remove

Comment on lines +91 to +92
$STD systemctl daemon-reload
msg_ok "Service Updated"
Copy link
Member

Choose a reason for hiding this comment

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

remove

@@ -0,0 +1,14 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get this structure from?!

Please refrain from generating random stuff with AI and submitting it here.

Read the actual guide I linked https://github.com/community-scripts/ProxmoxVE/wiki/CONTRIBUTING and use this JSON generator:
https://community-scripts.github.io/ProxmoxVE/json-editor

apt install -y git curl python3 python3-pip

# Clone and install Pinchflat
git clone https://github.com/kieraneglin/pinchflat /opt/pinchflat
Copy link
Member

Choose a reason for hiding this comment

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

As said we still don't do installs via git?
Didn't you understand this above?

Where is the virtual env install?

Look at the guide, this is nowwhere near what our install-scripts are like.
https://github.com/community-scripts/ProxmoxVE/wiki/CONTRIBUTING

@CrazyWolf13
Copy link
Member

I really suggest you start atually reading what I linked and not just generate random code you don't have a clue with AI and submit it here.

Everyone with a tiny idea of bash knows that this does not work like this.

@CrazyWolf13
Copy link
Member

Also remove that .DsStore file.

@AkuchiS
Copy link
Author

AkuchiS commented Jul 10, 2025

Oh wow, I am failing miserably and I thought co-pilot would help me fill in my gaps whilst I juggle kids and life etc. I appreciate the blunt and matter of fact feedback. Clearly, not my strong area. All I was hoping to achieve was to get "https://github.com/kieraneglin/pinchflat" self-contained docker across onto your VE Helper-Scripts as an lxc.

Originally, I looked other scripts that worked on my setup, tested it locally and was hoping that from the local instance to the global, it wouldn't be challenging. Boy, was I wrong.

I do love a challenge and, I do love to share my experiences. I truly appreciate your time and will work on this again in my local environment and see where I land.

Before I delve, this is "some" of my weaknesses?

Read the actual guide I linked https://github.com/community-scripts/ProxmoxVE/wiki/CONTRIBUTING and use this JSON generator:
https://community-scripts.github.io/ProxmoxVE/json-editor and,

Area | Action

ct/pinchflat.sh | Remove fetch_and_deploy and systemctl, fix source line
install/pinchflat-install.sh | Rewrite to follow proper Python VENV-based install (no git clone)
frontend/pinchflat.json | Recreate using JSON Editor
.DS_Store | Delete it, ignore it, commit that

@CrazyWolf13
Copy link
Member

Hi @AkuchiS
Thanks, I did not expect such a response!

Yeah feel free to take your time on this I get the feeling :)

And yes as pointed our we are meticulous when it comes to PR, but only like this we can hold up our standarts and deliver these well structured and safe to use scripts.

A little note on AI:
In our opinion, AI is not per-se a bad thing, it's more of a problem, when code is submitted and you notice OP not having a clue, about how it works, as it is impossible to work like you submitted.

btw. Claude and ChatGPT are much better in this than copilot.

@tremor021
Copy link
Member

tremor021 commented Jul 10, 2025

@AkuchiS Also, making a script for this app is basically just translating https://github.com/kieraneglin/pinchflat/blob/master/docker/selfhosted.Dockerfile into bunch of shell commands

Its not hard per se, it just requires a bit of testing and compliance with our contributor docs, as we require all scripts to follow specific rules

@AkuchiS
Copy link
Author

AkuchiS commented Jul 14, 2025

Totally appreciated..will keep going down the rabbit hole when I've the time :)

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.

3 participants