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

[WIP] DigitalOcean App Platform deployment button #215

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

slyapustin
Copy link

@slyapustin slyapustin commented Jan 6, 2025

@umputun Here is Draft PR for adding DO deployment button.

Some things to clarify/confirm:

  • Decide on where to put deployment button in the documentation (it's currently in the README.md).
  • This deployment is more suitable for Demo purpose (since App Platform does not provide persistent storage and changes will be lost on the instance restart). It should be fine for OpenAI integration AFAIK, since it should not store additional info on the disk.

Update things before merging:

  • Update DO referral code
  • Update link in the button to link to the main repo

Closes #205

@umputun
Copy link
Owner

umputun commented Jan 11, 2025

Looks interesting. Probably INSTALL.md will be a better place for this. If we are going to finish this thing, it needs a clear explanation of what limitations the lack of state will cause and to pass appropriate parameters as we deploy it. And I’m not really sure how all of this is supposed to work in practice, i.e., how exactly will the TG key get in? I see the template says "Replace with your Telegram token," but how will the user enter it practically? Will the DO deployment ask for this value?

@slyapustin
Copy link
Author

slyapustin commented Jan 14, 2025

@umputun

Probably INSTALL.md will be a better place for this.

yes, that make sense. I'll move docs there.

If we are going to finish this thing, it needs a clear explanation of what limitations the lack of state will cause and to pass appropriate parameters as we deploy it.

One of the limitation I'm seeing right now is that user/pass and custom settings made through UI will be lost on app restart.
That can be solved if there will be an option to use DB outside of that service (use library which supports SQLite as well as PG, MYSQL and etc). I saw you did a lot of the work for that in #203 but not sure if it's fully ready to be used with PostgreSQL or similar.

And I’m not really sure how all of this is supposed to work in practice

I was thinking about the use-case for that Telegram bot when someone wants to use it with OpenAI integration to protect group from bots. Do it need to have persistent file storage in this case as well?

I see the template says "Replace with your Telegram token," but how will the user enter it practically?

There is a step when you deploy to DO using that button to set/update environment variables.
https://docs.digitalocean.com/products/app-platform/how-to/create-apps/#environment

@umputun
Copy link
Owner

umputun commented Jan 14, 2025

#203 is still a work in progress. This is mostly done, but I need to add a bunch of tests for psql to make sure things are actually working correctly.

I was thinking about the use case for that Telegram bot when someone wants to use it with OpenAI integration to protect the group from bots. Does it need to have persistent file storage in this case as well?

tg-spam is fully functional without persistent storage; all the filters should work, not just OpenAI. The lack of persistency means that the things the bot learned won't be restored on restart, and this includes not only updated spam/ham samples but also the history of detected spam, locator info and the list of approved users. However, OpenAI itself doesn't use any storage, so there's nothing to lose here. And it is not just the OpenAI check— all other checks not based on samples will work just fine, i.e., emoji count, links, images, video, etc.

The part that won't make much sense in this configuration is the training, as it makes very little sense to train the bot's classifier with samples just to reset all of this to the default on restart.

@umputun
Copy link
Owner

umputun commented Jan 14, 2025

A small correction: #203 is actually in master already and should be good to go. However, it doesn't include other databases, just SQLite, which doesn't help here much. The part with "real databases" is not a PR yet, and this is the work in progress I have mentioned above.

@slyapustin
Copy link
Author

The part that won't make much sense in this configuration is the training, as it makes very little sense to train the bot's classifier with samples just to reset all of this to the default on restart.

Do you think having some flag for when it have persistent storage (sqlite on disc or DB as separate service) or not would help to save some resources and prevent user from trying to do some modifications in UI which would not persist?
For ex. based on that flag some alert can be shown in the UI which will explain that those changes will be lost on server restart.

I'm fine to do it with the current implementation and update later (with DB as a service or some feature flag).

@umputun
Copy link
Owner

umputun commented Jan 15, 2025

The system expects to have storage, and running with non-persistent storage (missing Docker volume mount) already shows warnings in the logs.

Making it work deliberately in storage-less mode is not a small amount of work, as many places have this basic expectation. I don't think we should even bother with this, as this limited deployment is just that - limited, at least for now. Showing this information on the install page or as part of the DO deployment details (if there is some description here) should be enough for the user to figure this out. Regarding saving resources - not much can be saved anyway by eliminating disk I/O, the bot is not using disk that heavily.

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.

[Feature Request] One-Click Deployment Script for Non-Technical Telegram Group Owners
2 participants