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

App Submission: Pingvin Share #1229

Merged
merged 14 commits into from
Oct 3, 2024
Merged

Conversation

MalteKiefer
Copy link
Contributor

App Submission

App name

Pingvin Share

256x256 SVG icon

logo
Sorry, no experience to create a SVG from an PNG

...

Gallery images

Pingvin_03
Pingvin_02
Pingvin_01

I have tested my app on:

  • umbrelOS on a Raspberry Pi
  • umbrelOS on an Umbrel Home
  • umbrelOS on Linux VM

Copy link
Contributor

@sharknoon sharknoon left a comment

Choose a reason for hiding this comment

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

Thank you so much @MalteKiefer for your awesome app submission! I have left some suggestions below.

permissions:
- STORAGE_DOWNLOADS
submitter: maltekiefer
submission:
Copy link
Contributor

Choose a reason for hiding this comment

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

I myself also forget to add the submission URL all the time :D

Suggested change
submission:
submission: https://github.com/getumbrel/umbrel-apps/pull/1229

category: files
name: Pingvin Share
version: "0.27.0"
tagline: Pingvin Share is self-hosted file sharing platform and an alternative for WeTransfer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Taglines should not end with a ..

Suggested change
tagline: Pingvin Share is self-hosted file sharing platform and an alternative for WeTransfer.
tagline: Pingvin Share is self-hosted file sharing platform and an alternative for WeTransfer

Comment on lines 16 to 17
ports:
- 6453:3000
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to expose this port when you are using the app_proxy. This way it is more secure :)

Suggested change
ports:
- 6453:3000

app_proxy:
environment:
APP_HOST: pingvin-share_web_1
APP_PORT: 6453
Copy link
Contributor

Choose a reason for hiding this comment

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

You can change this port to the internal port of the container.

Note

This port does not need to match the port in the umbrel-app.yml! The port there is the external port, this port is the port in this container network

Suggested change
APP_PORT: 6453
APP_PORT: 3000

pingvin-share/docker-compose.yml Show resolved Hide resolved

clamav:
restart: on-failure
image: clamav/clamav
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we have a very big problem: clamav/clamav does not run on the arm64 architecture. All apps need to run on arm64 as well as amd64. Maybe you can find a replacement for this? Also please don't forget to add the digest (@sha256...) for future images :)

Comment on lines 19 to 20
- "${APP_DATA_DIR}/data:/opt/app/backend/data"
- "${APP_DATA_DIR}/data/images:/opt/app/frontend/public/img"
Copy link
Contributor

Choose a reason for hiding this comment

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

Due to privilege issues on older umbrelOS devices, we need to add those directories to the app. Please create the following files:

  • pingvin-share/data/images/.gitkeep
  • pingvin-share/data/backend/.gitkeep

The problem is that on older versions of umbrelOS (again backwards compatibility) the permissions of automatically created repositories are wrong. By supplying a .gitkeep file, those directories are getting created by installing the app automatically.
Also maybe it would be better to save the backend data separately to prevent the clashing of files:

Suggested change
- "${APP_DATA_DIR}/data:/opt/app/backend/data"
- "${APP_DATA_DIR}/data/images:/opt/app/frontend/public/img"
- "${APP_DATA_DIR}/data/backend:/opt/app/backend/data"
- "${APP_DATA_DIR}/data/images:/opt/app/frontend/public/img"

@nmfretz
Copy link
Contributor

nmfretz commented Aug 29, 2024

@MalteKiefer friendly ping to see if you are still working on this. The main issue noted by @sharknoon is that the clamav image is only available for linux/amd64 architectures so the app would be broken for users running umbrelOS on a raspberry pi.

Edit: looks like the clamav container is optional: https://github.com/stonith404/pingvin-share/blob/main/docker-compose.yml
So for now, we could ship this without clamav until a multi-arch image becomes available Cisco-Talos/clamav#482

Edit 2: @sharknoon came across this multi-arch clamav-debian image from the same developers: https://hub.docker.com/r/clamav/clamav-debian/tags

@nmfretz
Copy link
Contributor

nmfretz commented Sep 6, 2024

Thinking about this more, it seems like Pingvin Share is really only useful when it is exposed to the internet. The easiest way to do this on umbrelOS right now is to:

  1. Install the Cloudflare Tunnel app
  2. Set up a Cloudflare account outside of umbrelOS and purchase+add a domain
  3. Set up a tunnel to the pingvin share web container, bypassing the app proxy.

This is certainly possible for technical users to do, but I think realistically what will happen is that an average user sees this cool looking file share app, installs it, and then becomes frustrated that it doesn't work out of the box.

I propose we convert this to draft and then launch it once umbrelOS has a simple way for users to expose apps to the internet (on the horizon). That way when this hits the official app store, there will be the option for it to work out-of-the-box for users.

In the meantime, technical users could still run Pingvin Share on their Umbrel by either:

@nmfretz nmfretz marked this pull request as draft September 6, 2024 10:49
@nmfretz nmfretz changed the title App Submission: Pingvin Share [Pending umbrelOS feature] - App Submission: Pingvin Share Sep 6, 2024
@dennysubke
Copy link
Contributor

@MalteKiefer
I have adopted pingvin share in my community appstore. Everything is going great. However, the app only makes sense in conjunction with Coudflare.

Screenshot Pingvin Share UmbrelStore

I propose we convert this to draft and then launch it once umbrelOS has a simple way for users to expose apps to the internet (on the horizon). That way when this hits the official app store, there will be the option for it to work out-of-the-box for users.

What do you mean by that, @nmfretz ?

@nmfretz
Copy link
Contributor

nmfretz commented Sep 24, 2024

Great to know it's working well on your community app store @dennysubke!

On revisiting this, I think we should send this to the app store with a note at the top of the app description saying that Pingvin Share needs to be exposed to the internet for full functionality. We do this for some other apps (e.g., Plausible), so it doesn't make sense to single out Pingvin Share for this.

We will get gallery assets prepared and I will finalize this submission.

What do you mean by that, @nmfretz ?

I can't give away too much at this point, but we have plans to allow users to easily expose apps to the internet. As noted above though, we shouldn't be waiting for this to bring this app to the app store. Thanks for your input @dennysubke!

@nmfretz nmfretz marked this pull request as ready for review September 24, 2024 01:15
@nmfretz nmfretz changed the title [Pending umbrelOS feature] - App Submission: Pingvin Share App Submission: Pingvin Share Sep 24, 2024
@getumbrel getumbrel deleted a comment from github-actions bot Oct 3, 2024
@getumbrel getumbrel deleted a comment from github-actions bot Oct 3, 2024
@nmfretz
Copy link
Contributor

nmfretz commented Oct 3, 2024

I have gone through and finalized this submission. I have tested on x86 and arm64, including exposing Pingvin Share to the web using a Cloudflare tunnel.

image

@nmfretz nmfretz merged commit 84a2919 into getumbrel:master Oct 3, 2024
1 check passed
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.

4 participants