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 NGINX ProxyManager (Install / Update) #763

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

Conversation

MickLesk
Copy link
Member

Description

Reverted deprecated pnpm in frontend, switched to official yarn

Fixes #761

Type of change

Please check the relevant option(s):

  • Bug fix (non-breaking change that resolves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (a fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts.)

Prerequisites

The following efforts must be made for the PR to be considered. Please check when completed:

  • Self-review performed (I have reviewed my code, ensuring it follows established patterns and conventions)
  • Testing performed (I have tested my changes, ensuring everything works as expected)
  • Documentation updated (I have updated any relevant documentation)

Additional Information (optional)

Provide any additional context or screenshots about the feature or fix here.

Related Pull Requests / Discussions

If there are other pull requests or discussions related to this change, please link them here:

  • Related PR #

@MickLesk MickLesk requested a review from a team as a code owner December 10, 2024 08:47
@github-actions github-actions bot added the update script A change that updates a script label Dec 10, 2024
@havardthom
Copy link
Contributor

What is the bug? Script installs fine here.
image

$STD pnpm install
$STD pnpm upgrade
$STD pnpm run build
$STD yarn cache clean --silent --force
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this? Where is yarn installed? pnpm install section is not removed? tteck/Proxmox@edf3803#diff-e676f31f25717f8123cc0904ec5973dee3dc53b86e2b0ff7bec0eeb72c6ce82fR132

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, you can run the bash script against the develop Branch

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

✓ Stopped Services
✓ Cleaned Old Files
✓ Downloaded NPM v2.12.1
✓ Setup Enviroment
/ Building Frontend
[ERROR] in line 143: exit code 0: while executing command yarn cache clean --silent --force &> /dev/null

running against the dev branch ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Lol. Work on my systems fine :D fck NPM

Copy link
Contributor

@havardthom havardthom Dec 11, 2024

Choose a reason for hiding this comment

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

https://github.com/NginxProxyManager/nginx-proxy-manager/blob/b3de76c945b087b9a585faff128ca655170f8ea1/docker/Dockerfile#L44-L47

That is backend install, not the same as frontend install which you are changing in this PR. Frontend in that dockerfile is prebuilt (COPY frontend/dist /app/frontend)

@MickLesk
Copy link
Member Author

What is the bug? Script installs fine here.
image

Install works, Update not. Because pnpm load deprecated Files, for first Install is this Not a big Problem, but with Update

@rowie
Copy link

rowie commented Dec 11, 2024

Why is this thing dying with every update?

@MickLesk
Copy link
Member Author

Why is this thing dying with every update?

This script just sucks. There's something new every time

@havardthom
Copy link
Contributor

havardthom commented Dec 11, 2024

Update works fine here also, service runs fine afterwards.
The issue linked in the description had an error in install script which I can not reproduce (probably one time issue for that user?): [ERROR] in line 146: exit code 0: while executing command $STD pnpm run build
Does anyone have issues when installing/updating current script?
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update script A change that updates a script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nginx Proxy Manager LXC Error in line 146
3 participants