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

Farm trait #2626

Merged
merged 6 commits into from
Mar 19, 2024
Merged

Farm trait #2626

merged 6 commits into from
Mar 19, 2024

Conversation

nazar-pc
Copy link
Member

This is the first step towards #2402

Specifically this set of changes introduces a series of refactoring that end with farm command only working with an abstract farm (outside of initialization). This will allow to introduce remote farms in the future that implement the same API.

Most of the commits are just renaming and moving things around, but implementation of new traits also required some internal refactoring to make APIs properly async.

Code contributor checklist:

Copy link
Contributor

@shamil-gadelshin shamil-gadelshin left a comment

Choose a reason for hiding this comment

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

It's possible to optimize the remote cache (to query in batches) and API will diverge but we can do that later.

@nazar-pc
Copy link
Member Author

It's possible to optimize the remote cache (to query in batches) and API will diverge but we can do that later.

Can you elaborate on that?

@nazar-pc nazar-pc added this pull request to the merge queue Mar 19, 2024
@shamil-gadelshin
Copy link
Contributor

It's possible to optimize the remote cache (to query in batches) and API will diverge but we can do that later.

Can you elaborate on that?

If I understand the changes correctly - you're preparing the code for different cache implementations - local and remote (among other things). However, remote and local APIs are not fully interchangeable in most cases because of the non-zero cost of API invocation in the remote case. For example, in the network environment instead of get_piece_by_id we can get pieces from the cache in batches to optimize it. I don't think this will be an issue for the first version, especially in LAN setup but it might produce unexpected behavior in WAN setup.

@nazar-pc
Copy link
Member Author

I see. This feature, at least the way I see it, is only for LAN usage.

@shamil-gadelshin
Copy link
Contributor

I see. This feature, at least the way I see it, is only for LAN usage.

Yes, it is. But there is nothing to the best of my knowledge will prevent users from adding WAN-remote peers.

Merged via the queue into main with commit ddda10c Mar 19, 2024
9 checks passed
@nazar-pc nazar-pc deleted the farm-trait branch March 19, 2024 11:10
@nazar-pc
Copy link
Member Author

Well, that will be on them I guess. There are many ways to use software in a way that it wasn't designed for, but it doesn't mean we should optimize for those. We'll see how it goes.

@GuyCre8ive
Copy link

In my framework I have what I call an "Apex" host that checks for updates and other data on my network over the Internet. I then allow all my other systems to define the Apex host upon setup and they check that host before checking the Internet. I've read people complaining about their ISPs giving them problems when trying to use this project so this seems like a really good idea if it can be done easily. I just setup several nodes for Subspace and it's using a ton of bandwidth so I can see why some smaller ISPs might have a problem with that. Just trying to help, keep up the good work though!

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.

3 participants