Skip to content
This repository was archived by the owner on Jan 21, 2022. It is now read-only.

Conversation

skobkin
Copy link
Contributor

@skobkin skobkin commented Oct 15, 2019

In this PR I implemented simple database engine for PostgreSQL.
It implements pkg/persistence/interface.go partially only for magneticod part.

It uses (and slightly refactors) the same format as stdout engine.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

@skobkin skobkin marked this pull request as ready for review October 15, 2019 20:11
"github.com/pkg/errors"
)

var beanstalkdWriteOnlyError = errors.New("This dummy storage engine (\"beanstalkd\") is write-only")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably we need to make common error for this type of "not implemented" or "magneticod only" errors like I did with SimpleTorrentSummary in interface.go.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could create a WriteOnly interface that implements all the functions which read on data (GetNumberOfTorrents, get Torrent, Getdata etc.). Then Beanstalk (or any other kind of write-only db like StdOut) could build on it implementing the remaining "write" function necessary to get to a full Database implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lruggieri Totally agree with this idea. But I'm not ready to do this refactoring myself.

Copy link
Contributor

Choose a reason for hiding this comment

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

@skobkin I did a PR on your own branch, implementing this kind of interface. See if you like it.

Copy link
Contributor Author

@skobkin skobkin Nov 13, 2019

Choose a reason for hiding this comment

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

@lruggieri I saw it. Looks good for me. But it's would be better to propose such changes to the upstream and then if it's approved I can rebase my PR on them.

Proposing both in my PR would not be good for segregation of proposed changes.

You can implement the same for stdout engine and then I'll pull it from upstream.

@lruggieri
Copy link
Contributor

I guess every new DB should also have a Docker image and the main magneticod image should depend on it

@skobkin
Copy link
Contributor Author

skobkin commented Oct 26, 2019

@lruggieri I don't think so. At least with current magnetico architecture it would be rather complicated to separate all engines from the core tool.

Do you have understanding how to implement your scheme?

@lruggieri
Copy link
Contributor

@lruggieri I don't think so. At least with current magnetico architecture it would be rather complicated to separate all engines from the core tool.

Do you have understanding how to implement your scheme?

If you want to make this the main DB for the project I'd say to dockerize the DB itself, sharing the data folder and then put a depends_on POSTGRES_SERVICE in magneticod service.

If, instead, the purpose is letting the user choose between several DB systems...well, in that case, I would still dockerize every DB and maybe not putting the dependency in the compose file. In that case there should however be a nice explanation in the README about how the container of the DB of choice should be run before starting the main program.

@skobkin
Copy link
Contributor Author

skobkin commented Oct 26, 2019

@lruggieri Huh. I thought that you're talking about magneticod code itself.

If you talking about how to run it in the Docker then this PR is not about it at all.

BTW I still don't fully understand what's you're trying to say.

If you want to make this the main DB for the project I'd say to dockerize the DB itself

What's "this"? If "this" is the magneticod then you already have two *.Dockerfile in the root of this repository to build magneticod and magneticow.

If "this" is Beanstalkd then this PR is not about how to run Beanstalkd. It's just implementing persistence/storage engine for magneticod to be able to put every discovered torrent as new job in Beanstalkd.

How to run any storage back-end is your choice. This PR is only about supporting one of the storages.

I don't think that magnetico repository should have documentation about every supported storage engine. IMHO magnetico should have documentation only about default SQLite engine which in most cases should work fine without any interaction from the user.

If you want to use any other storage you must understand why you need it and how to manage it. It's not magnetico's responsibility to teach every user how to handle every storage.

@lruggieri
Copy link
Contributor

@skobkin you are right, reading "In this PR I implemented simple database engine for PostgreSQL." in the first message I actually thought this PR was actually implementing the Postgres DB interface.
My bad, I should gone through the commit changes first.

This is actually a queue to put SimpleTorrentSummary objects there to be used, right? I still don't fully see the purpose though. Who should use those queued jobs?

@skobkin
Copy link
Contributor Author

skobkin commented Oct 26, 2019

This is actually a queue to put SimpleTorrentSummary objects there to be used, right?

Yes. I've just used already existing out structure from stdout engine but moved it to the interface and renamed for common use.

Who should use those queued jobs?

Anyone who want to process indexed torrents on their own or save them in their own database schema.

For example using beanstalk engine you can implement #223 in your application logic not relying on magneticod.

Also you can implement almost instant torrent availability notification system for your users. To implement something like these you'll need to use triggers or perpetual polling from the SQLite or PostgreSQL database.

So basically it's up to your imagination.

@lruggieri
Copy link
Contributor

@skobkin
I was thinking that magnetico should probably not implement internally any other DB interface other than SQLite, and just adding a boolean option for enabling the queue or not.
Then the whole queue system should be properly dockerized (queue included: installation, ports exporting, settings and stuff) and any user who wants to implement an external DB (like me with Elasticsearch + Kibana) would just have to connect to the queue on the container.

In this instance the queue should not even be considered a Database anymore.

@boramalper boramalper merged commit 1e3604c into boramalper:master Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants