-
Notifications
You must be signed in to change notification settings - Fork 455
App submission: sat.watch #2723
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jpcummins!
Thanks for the app submission! 🚀
I added some comments on what should be adapted to get the app into the store.
One thing I noticed is that I was not able to add any addresses without providing a valid smtp config. Even after adding a webhook I still got an error about missing smtp. Is this expected?
About the admin user, as I did not see a way to create another user via the UI, it would make sense to adapt the hook from a post-install to a pre-start script to make sure the user is created again if it is deleted by mistake. Otherwise a reinstall would be needed.
@al-lac Thank you for taking the time to review my app submission—I really appreciate the thoughtful feedback! I should have some updates ready for review next week.
For this initial release, I intentionally left out user management based on the assumption that Umbrel operates as a single-user application (since it runs behind the App Proxy). As a result, the script currently creates only a single user, and there’s no UI to update or delete this user. However, I’m happy to add user management if that’s preferred. In future releases, when I implement user management, I’ll make sure to move user creation to the pre-start phase as you suggested. Please let me know if my assumptions about the single-user model are incorrect or if you’d prefer that I include user management in this release. |
fix: PR feedback, update satwatch version
@jpcummins, thanks for making the adaptions. It seems to be possible to delete the satwatch user from the Web UI. Adding a pre-start script just makes sure that the user is added back in case that happens. A pre-start script is preferred because if the post-install fails, it will never run again. With a pre-start script you can just restart the app to run it again. Could do something similar to |
@al-lac Thanks again for the helpful feedback—and apologies for the confusion on my end regarding user management. You’re absolutely right; there’s a delete account button, and I completely blanked on that. I’ve now implemented the changes and moved user creation to a pre-start script. Instead of using a file, I updated the user creation script to be idempotent, which I think makes the process a bit more robust. You can see the changes here: I’ve tested this on both umbrel-dev and a Raspberry Pi. I confirmed that deleting the admin user and restarting correctly recreates the user, and restarting without deleting the user doesn’t result in duplicates. I’ve also added new screenshots for the latest version of the app: Screenshot 1 Let me know if there’s anything else you’d like me to adjust. |
Really great job @jpcummins! 👏 Tested it and it works well! Also re-added the user after I deleted it after a restart of the app. One thing that would still be nice is if you could let the DB run as user Will start to get the assets made now! |
🎉 Linting finished with no errors or warnings 🎉Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store. |
@al-lac Database permissions updated. 🫡 It turned out to be a bit tricky—initially, the container failed to start due to permission errors. After some debugging, I realized the issue was that the volume mount point didn’t exist and was being created at startup by the container with root permissions. As a result, user 1000 couldn’t access the data directory. I fixed this by adding a /data/db/.gitkeep file to ensure the directory exists ahead of time with the correct permissions. You might add this as a warning to your linter -- make sure volume mount directories exist if referenced. Also, I’ve updated the app display name from "satwatch" to "sat.watch" to keep things consistent with the documentation and website. |
@jpcummins great job on the adaptions! 🚀 Actually the linter already has a check for that, guess both of us just missed that, see here for a previous job result: https://github.com/getumbrel/umbrel-apps/actions/runs/14980416434 |
Hey @al-lac, just checking in. Let me know if you need any additional screenshots, assets, or anything. Thanks! |
Hey @jpcummins, assets are being prepared right now. Should be ready pretty soon! :-) |
App Submission
sat.watch
sat.watch monitors your Bitcoin and sends real-time alerts for mempool and block confirmation activity.
Icon
SVG Icon
Gallery images
Screenshots
I could really use some help on the gallery images. If you need additional screenshots, please let me know.
...
I have tested my app on: