feat(proxy): peer branches #678
Conversation
Moving the revisions function into source so that it only operates over a Browser. The peer logic gets moved to handler and the information gets passed down. As well as this, we extend the branches endpoint so that it can take a PeerId if we want to look at a peer's list of branches. We test this in the revisions test.
|
I left this as a draft because I built on top of the other PR. But it's pretty much good to review. |
Co-authored-by: Rūdolfs Ošiņš <[email protected]>
proxy/src/http/source.rs
Outdated
| "project_id", | ||
| "ID of the project the blob is part of", | ||
| )) | ||
| .and(warp::filters::query::query::<PeerQuery>()) |
There was a problem hiding this comment.
Shouldn't it be called BranchesQuery? Since it's the query string for /branches?
proxy/src/coco/source.rs
Outdated
| #[serde(rename_all = "camelCase")] | ||
| pub struct UserRevisions { | ||
| /// Owner of the repo. | ||
| pub(crate) identity: identity::Identity, |
There was a problem hiding this comment.
Just curious - why are these only visible in the crate?
There was a problem hiding this comment.
Is this a best practice we should be following elsewhere too?
There was a problem hiding this comment.
They were private, so I went one level up. I suppose since proxy is actually the only consumer of this library it could just be pub.
What's your stance on this @xla?
There was a problem hiding this comment.
We don't really use crate visibility anywhere else at the moment. While we should be diligent about our library (i.e. the everything under the lib.rs mod tree) - we don't really have been so far. It's fair to use pub for now and clean up the lib once it has more applications than just being consumed in our very own beloved main.rs.
proxy/src/coco/source.rs
Outdated
| #[serde(rename_all = "camelCase")] | ||
| pub struct UserRevisions { | ||
| /// Owner of the repo. | ||
| pub(crate) identity: identity::Identity, |
There was a problem hiding this comment.
We don't really use crate visibility anywhere else at the moment. While we should be diligent about our library (i.e. the everything under the lib.rs mod tree) - we don't really have been so far. It's fair to use pub for now and clean up the lib once it has more applications than just being consumed in our very own beloved main.rs.
proxy/src/coco/source.rs
Outdated
| /// Bundled response to retrieve both branches and tags for a user repo. | ||
| #[derive(Debug, Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct UserRevisions { |
There was a problem hiding this comment.
Do we have any other form of Revisions? My question aims at for our domain which tries to capture the source tree exploration, we always have the dimensions of identity if I understand correctly, so could this be just named Revisions?
There was a problem hiding this comment.
We have Revision, which is an enum over Tag, Branch or Sha. So I wouldn't say that this is multiple of those, technically. Not sure what would capture this concept better 🤷 Maybe Revisions with docs
proxy/src/coco/source.rs
Outdated
| use crate::error; | ||
| use crate::identity; |
There was a problem hiding this comment.
I highlighted this in another PR, this import shows that we haven't really figured out the direction of dependencies. At the core of our library are the protocol wrappers and they should be consumed by outer layers (identity, project, session) which ultimately is packaged in our APIs (HTTP for now). I'd like to challenge how we can make sure that identity is not imported here, maybe the concept of revisions scoped by identity doesn't belong down here in the coco machine room.
There was a problem hiding this comment.
I forgot about generics. No more modules :)
| @@ -70,7 +70,7 @@ where | |||
| .and(http::with_peer(peer)) | |||
| .and(http::with_shared(registry)) | |||
| .and(http::with_store(store)) | |||
| .and(document::param::<String>( | |||
| .and(document::param::<coco::Urn>( | |||
* Renamed UserRevisions to Revisions. * Removed Identity and replaced with peer_id, user fields. * Parameterise over P and U so that we don't import librad modules.
|
WIP Update: I parameterised but the output isn't exactly what we want for the front end. Will finish up tomorrow. |
|
@xla: should have a cleaner bound of separation now. Lemme know what you think :) |
Co-authored-by: Alexander Simmerl <[email protected]>
Moving the revisions function into source so that it only operates over
a Browser. The peer logic gets moved to handler and the information gets
passed down.
As well as this, we extend the branches endpoint so that it can take a
PeerId if we want to look at a peer's list of branches. We test this in the revisions test.