Summary: This change adds wire types for the history API in the most straightforward way possible. In a future change, I'll move the `WireHistoryEntry` / `HistoryResponseChunk` conversion logic into the `ToApi` implementation. This implementation also doesn't add per-item errors, or standardize `history` to match the protocol evolution standards used by `trees`.
Reviewed By: kulshrax
Differential Revision: D24342046
fbshipit-source-id: d46403a823f2a1e89ad9d6d2074241d8bfe4810e
Summary:
Removes top-level metadata from `TreeEntry` and add a new, specialized type for carrying child entry metadata, `TreeChildEntry`. This change also fully removes the `revisionstore_types::Metadata` from EdenAPI trees, which is only used on files.
In a follow up change I'll optimize handlings of paths / path segments in `TreeChildEntry` keys. Right now they need to be joined manually.
Reviewed By: kulshrax
Differential Revision: D24434716
fbshipit-source-id: d0739471b1f6cef58b435e10b5fb774bfb08f7f6
Summary: Rather than silently dropping entries which cannot be fetched, this change has the `WireTreeEntry` type carry optional error information, allowing it to be (de)serialized to / from `Result<TreeEntry, EdenApiServerError>` instead of a bare `TreeEntry`. Currently, handling of these failures is up to the individual application code, but it might be useful to introduce utility functions to drop failed entries and log errors.
Reviewed By: kulshrax
Differential Revision: D24315399
fbshipit-source-id: 94e4593b77cf2dc12d0dcc93d174c8a4eda95344
Summary: In D24447404, I provided some utility functions that allowed me to avoid constructing and/or passing around a ConfigStore. Remove those functions and fix up the code to run.
Reviewed By: krallin
Differential Revision: D24502692
fbshipit-source-id: 742dbc54fbcf735895d6829745b9317af14dfa0b
Summary: It's designed as a singleton store for normal use - rather than have lots of ways to get different config stores, let's use one global store
Reviewed By: krallin
Differential Revision: D24447404
fbshipit-source-id: 6ecc46351b14794471f654cec98527a11a93d3ef
Summary: Introduces fetching of child entry IDs, and child file metadata for a specified tree manifest ID. The aux data lookup will only be performed if `with_file_metadata` is set, which is actually kind of wrong. Instead `with_children` from the wire type should be exposed in the API request type, and `with_*_metadata` should be hidden or used for data other than the child entry `Key`s.
Reviewed By: kulshrax
Differential Revision: D23886678
fbshipit-source-id: 0cba72cea7be47ae3348a406d407a19b60976c0c
Summary: Add a new "user" column to the EdenAPI server's Scuba data.
Reviewed By: krallin
Differential Revision: D24153765
fbshipit-source-id: 95a3177d9283e5b0e3f47e7f42a1de5142049b99
Summary:
Introduce separate wire types to allow protocol evolution and client API changes to happen independently.
* Duplicate `*Request`, `*Entry`, `Key`, `Parents`, `RepoPathBuf`, `HgId`, and `revisionstore_types::Metadata` types into the `wire` module. The versions in the `wire` module are required to have proper `serde` annotations, `Serialize` / `Deserialize` implementations, etc. These have been removed from the original structs.
* Introduce infallible conversions from "API types" to "wire types" with the `ToWire` trait and fallible conversions from "wire types" to "API types" with the `ToApi`. API -> wire conversions should never fail in a binary that builds succesfully, but wire -> API conversions can fail in the case that the server and client are using different versions of the library. This will cause, for instance, a newly-introduced enum variant used by the client to be deserialized into the catch-all `Unknown` variant on the server, which won't generally have a corresponding representation in the API type.
* Cleanup: remove `*Response` types, which are no longer used anywhere.
* Introduce a `map` method on `Fetch` struct which allows a fallible conversion function to be used to convert a `Fetch<T>` to a `Fetch<U>`. This function is used in the edenapi client implementation to convert from wire types to API types.
* Modify `edenapi_server` to convert from API types to wire types.
* Modify `edenapi_cli` to convert back to wire types before serializing responses to disk.
* Modify `make_req` to use `ToWire` for converting API structs from the `json` module to wire structs.
* Modify `read_res` to use `ToApi` to convert deserialized wire types to API types with the necessary methods for investigating the contents (`.data()`, primarily). It will print an error message to stderr if it encounters a wire type which cannot be converted into the corresponding API type.
* Add some documentation about protocol conventions to the root of the `wire` module.
Reviewed By: kulshrax
Differential Revision: D23224705
fbshipit-source-id: 88f8addc403f3a8da3cde2aeee765899a826446d
Summary: Make `StreamBody` accept a `Stream` of `Bytes` instead of a `TryStream` of `Bytes`. This means that applications returning streaming responses will be forced to deal with errors prior to returning the response.
Reviewed By: krallin
Differential Revision: D23780216
fbshipit-source-id: dbad61947ef23bbfc4edf3d286ad0218c1859d87
Summary: Use the new `ForwardErr` stream combinator to log errors that occur during a streaming response. Right now, they are just printed to stderr, but in the future we should also do other things such as logging them to Scuba. This diff supersedes the approach from D22720957.
Reviewed By: krallin
Differential Revision: D23780215
fbshipit-source-id: 8d2267f1166e665a62a167a6d95bb0b1797b5767
Summary:
The `gotham_ext::response` module was getting a bit large, so this diff moves `ContentMeta`, `ContentStream`, and `CompressedContentStream` into a new submodule, alongside the contents of the old `content_encoding` module. This way, the `response` module remains entirely centered around the `TryIntoResponse` trait (and the various body structs that implement that trait).
Later diffs in this stack will be adding an additional layer between the content streams and the body structs, at which point it probably doesn't make sense to have these right next to each other. Splitting them out now will allow for better code organization going forward.
Reviewed By: krallin
Differential Revision: D23777492
fbshipit-source-id: 86e598dcb37578d3b22217a2a65f1bde84d72215
Summary:
In preparation of moving away from SSH as an intermediate entry point for
Mononoke, let Mononoke work with newly introduced Metadata. This removes any
assumptions we now make about how certain data is presented to us, making the
current "ssh preamble" no longer central.
Metadata is primarily based around identities and provides some
backwards-compatible entry points to make sure we can satisfy downstream
consumers of commits like hooks and logs.
Simarly we now do our own reverse DNS resolving instead of relying on what's
been provided by the client. This is done in an async matter and we don't rely
on the result, so Mononoke can keep functioning in case DNS is offline.
Reviewed By: farnz
Differential Revision: D23596262
fbshipit-source-id: 3a4e97a429b13bae76ae1cdf428de0246e684a27
Summary:
This adds support for compressing responses in the LFS Server, based on what
the client sent in `Accept-Encoding`. The compression changes are fairly
simple. Most of the codes changes are around the fact that when we compress,
we don't send a Content-Length (because we don't know how long the content will
be).
Note that this is largely implemented in StreamBody. This means it can be used
for free by the EdenAPI server as well. The reason it's in there is because we
need to avoid setting the Content-Length when compression is going to be used
(`StreamBody` is what takes charge for doing this). This also exposes a
callback to get access to the stream post-compression, which also needs to be
exposed in `StreamBody`, since that's where compression happens.
Reviewed By: aslpavel
Differential Revision: D23652334
fbshipit-source-id: 8f462d69139991c3e1d37f392d448904206ec0d2
Summary: Add a Scuba handler for the EdenAPI server, which will allow the server to log custom columns to Scuba via `ScubaMiddleware`. For now, the only application specific columns are `repo` and `method`.
Reviewed By: krallin
Differential Revision: D23619437
fbshipit-source-id: f08aaf9c84657b4d92f1a1cfe5f0cb5ccd408e5e
Summary: Add `OdsMiddleware` to the EdenAPI server to log various aggregate request stats to ODS. This middleware is directly based on the `OdsMiddleware` from the LFS server (which unfortunately can't be easily generalized due to the way in which the `stats` crate works).
Reviewed By: krallin
Differential Revision: D23619075
fbshipit-source-id: d361c73d18e0d1cb57347fd24c43bdb68fb7819d
Summary:
Add a new `HandlerInfo` struct that can be inserted into the request's `State` by each handler to provide information about the handler to post-request middleware. (This includes things like which handler ran, which repo was queried, etc.)
A note on design: I did consider adding these fields to `RequestContext` (similar to how it's done in the LFS server), but that proved somewhat problematic in that it would require using interior mutability to avoid forcing each handler to borrow the `RequestContext` mutably (and therefore prevent it from borrowing other state data). The current approach seems preferable to adding a `Mutex` inside of `RequestContext`.
Reviewed By: krallin
Differential Revision: D23619077
fbshipit-source-id: 911806b60126d41e2132d1ca62ba863c929d4dc9
Summary: Add Scuba logging using `ScubaMiddleware` from `gotham_ext`. Each request will be logged to the Scuba dataset specified by the `--scuba-dataset` flag, as well as optionally to the log file specified by `--scuba-log-file`.
Reviewed By: sfilipco
Differential Revision: D23547668
fbshipit-source-id: e6cd88ad729a40cf45b63538f7481ee098ea12dc
Summary: Add `LogMiddleware` to the EdenAPI server, which will print a log message whenever a request is received or has completed.
Reviewed By: DurhamG
Differential Revision: D23299902
fbshipit-source-id: f44ef1b01692f0e4f9b109917fcee89a84ca4208
Summary: Use `LoadMiddleware` to track the number of outstanding requests in the server.
Reviewed By: DurhamG
Differential Revision: D23298415
fbshipit-source-id: bdcdb0f657d8deac593d356c87ac0d8d3f39e322
Summary:
To reduce the size over the wire on cases where we would be traversing the
changelog on the client, we want to allow the endpoint to return a whole parent
chain with their hashes.
Reviewed By: kulshrax
Differential Revision: D23456216
fbshipit-source-id: d048462fa8415d0466dd8e814144347df7a3452a
Summary:
Renaming all the LocationToHash related structures to CommitLocationToHash.
This is done for consistency. I realized the issue when the command for reading
the request from cbor was not what I was expecting it to be. The reason was that
the commit prefix was used inconsistently for LocationToHash.
Reviewed By: kulshrax
Differential Revision: D23456221
fbshipit-source-id: 0181dcaf81368b978902d8ca79c5405838e4b184
Summary: This function used to be longer before AclChecker was replaced with PermissionsChecker. Now the function is a one-liner, so it doesn't make sense to keep it as a separate function.
Reviewed By: sfilipco
Differential Revision: D23304899
fbshipit-source-id: 23e8c4b2334cdbff21ca336aecedf6ba6c466f99
Summary: Use bookmarks_movement to implement the bookmark move in repo_move_bookmark.
Reviewed By: StanislavGlebik
Differential Revision: D23222562
fbshipit-source-id: 31249411d9521823f90248f459eb34ed4e2faea5
Summary: We will shortly need a `HookManager` in the write methods of the source control service. Add one to `mononoke_api::Repo`
Reviewed By: StanislavGlebik
Differential Revision: D23077552
fbshipit-source-id: e1eed3661fe26a839e50ac4d884f4fadf793dbbb
Summary:
The primary change is in `eden/scm/lib/edenapi/types`:
* Split `DataEntry` into `FileEntry` and `TreeEntry`.
* Split `DataError` into `FileError` and `TreeError`. Remove `Redacted` error variant from `TreeError` and `MaybeHybridManifest` error variant from `FileError`.
* Split `DataRequest`, `DataResponse` into appropriate File and Tree types.
* Refactor `data.rs` into `file.rs` and `tree.rs`.
* Lift `InvalidHgId` error, used by both File and Tree, into `lib.rs`.
* Bugfix: change `MaybeHybridManifest` to be returned only for hash mismatches with empty paths, to match documented behavior.
Most of the remaining changes are straightforward fallout of this split. Notable changes include:
* `eden/scm/lib/edenapi/tools/read_res`: I've split the "data" commands into "file" and "tree", but I've left the identical arguments sharing the same argument structs. These can be refactored later if / when they diverge.
* `eden/scm/lib/types/src/hgid.rs`: Moved `compute_hgid` from `eden/scm/lib/edenapi/types/src/data.rs` to as a new `from_content` constructor on the `HgId` struct.
* `eden/scm/lib/revisionstore/src/datastore.rs`: Split `add_entry` method on `HgIdMutableDeltaStore` trait into `add_file` and `add_tree` methods.
* `eden/scm/lib/revisionstore/src/edenapi`
* `mod.rs`: Split `prefetch` method on `EdenApiStoreKind` into `prefetch_files` and `prefetch_trees`, which are given a default implementation that fails with `unimplemented!`.
* `data.rs`: Replace blanket trait implementations for `EdenApiDataStore<T>` with specific implementations for `EdenApiDataStore<File>` and `EdenApiDataStore<Tree>` which call the appropriate fetch and add functions.
* `data.rs` `test_get_*`: Replace dummy hashes with real hashes. These tests were only passing due to the hash mismatches (incorrectly) being considered `MaybeHybridManifest` errors, and allowed to pass.
Reviewed By: kulshrax
Differential Revision: D22958373
fbshipit-source-id: 788baaad4d9be20686d527f819a7342678740bc3
Summary:
Matches the `getcommitdata` SSH endpoint.
This is going to be used to remove the requirement that client repostories
need to have all commits locally.
Reviewed By: krallin
Differential Revision: D22979458
fbshipit-source-id: 75d7265daf4e51d3b32d76aeac12207f553f8f61
Summary:
Eden api endpoint for segmented changelog. It translates a path in the
graph to the hash corresponding to that commit that the path lands on.
It is expected that paths point to unique commits.
This change looks to go through the plumbing of getting the request from
the edenapi side through mononoke internals and to the segmented changelog
crate. The request used is an example. Follow up changes will look more at
what shape the request and reponse should have.
Reviewed By: kulshrax
Differential Revision: D22702016
fbshipit-source-id: 9615a0571f31a8819acd2b4dc548f49e36f44ab2
Summary: Use `create_getpack_v2_blob` instead of `create_getpack_v1_blob` for fetching file content because the former also provides metadata, which is required to support LFS.
Reviewed By: quark-zju
Differential Revision: D22564950
fbshipit-source-id: 2835160a9dfd18b80cd13e4a5dbcf6f4ce2f4579
Summary:
When we change `CommitSyncConfig`, we want to not have to restart `scs` servers, and instead have them pick up the new config by using `LiveCommitSyncConfig`.
This diff turned out larger than I expected, mainly due to the need to introduce various things around `TestLiveCommitSyncConfig`:
- `TestLiveCommitSyncConfig`: the trait implementer to use in `mononoke_api::Repo`
- `TestLiveCommitSyncConfigSource`: the helper struct to keep around for new values injection (very similar to how our `ConfigStore` has an inner `ConfigSource`, which can also be `TestSource`, but here injected values can be `CommitSyncConfig` instead of JSON).
- all the places in integration tests, where `setup_configerator_configs` is now needed (I plan to start setting up configerator configs always in a separate diff, as it is cheap)
Here are the explanations for a few things I think may be not immediately obvious:
- I removed the `Clone` bound from `LiveCommitSyncConfig` trait, as `Clone` traits [cannot be made into trait objects](https://doc.rust-lang.org/book/ch17-02-trait-objects.html#object-safety-is-required-for-trait-objects)
- `TestLiveCommitSyncConfigSource` vs `TestLiveCommitSyncConfigSourceInner` discrepancy is to ensure nobody should instantiate `TestLiveCommitSyncConfigSourceInner` outside of `live_commit_sync_config/src`
- I am aware of the ugly discrepancy between the main `--mononoke-config-path`, which is used to load initial configuration and can be both a file-based and a configerator-based config; and `--local-configerator-path`, used to override config sources for `Tunables` and `LiveCommitSyncConfig`. Ideally these two should just be unified somehow, but that is a little out of scope of this diff (I've already added it to the dirt doc though).
- in `mononoke_api::Repo` there are methods `new_test_xrepo` and `new_test_common`, which changed from maybe accepting just a `CommitSyncConfig` to now maybe accepting both `CommitSyncConfig` and `LiveCommitSyncConfig`. It can be made a bit cleaner: I can just query `CommitSyncConfig` from `LiveCommitSyncConfig` in `new_test_common` and avoid having two args. I was too lazy to do this, lmk if you feel strongly about it.
Reviewed By: StanislavGlebik
Differential Revision: D22443623
fbshipit-source-id: 0d6bbda2223e77b89cc59863b703db5081bcd601
Summary:
Add a metadata field to `read_res` containing a `revisionstore::Metadata` struct (which contains the object size and flags). The main purpose of this is to support LFS, which is indicated via a metadata flag.
Although this change affects the `DataEntry` struct which is serialized over the wire, version skew between the client and server should not break things since the field will automatically be populated with a default value if it is missing in the serialized response, and ignored if the client was built with an earlier version of the code without this field.
In practice, version skew isn't really a concern since this isn't used in production yet.
Reviewed By: quark-zju
Differential Revision: D22544195
fbshipit-source-id: 0af5c0565c17bdd61be5d346df008c92c5854e08
Summary: D22381744 updated the version of `futures` in third-party/rust to 0.3.5, but did not regenerate the autocargo-managed Cargo.toml files in the repo. Although this is a semver-compatible change (and therefore should not break anything), it means that affected projects would see changes to all of their Cargo.toml files the next time they ran `cargo autocargo`.
Reviewed By: dtolnay
Differential Revision: D22403809
fbshipit-source-id: eb1fdbaf69c99549309da0f67c9bebcb69c1131b
Summary: If a value is above cachelib limit let's try to compress it.
Reviewed By: krallin
Differential Revision: D22139644
fbshipit-source-id: 9eb366e8ec94fe66529d27892a988b035989332a
Summary:
Remove unused dependencies for Rust targets.
This failed to remove the dependencies in eden/scm/edenscmnative/bindings
because of the extra macro layer.
Manual edits (named_deps) and misc output in P133451794
Reviewed By: dtolnay
Differential Revision: D22083498
fbshipit-source-id: 170bbaf3c6d767e52e86152d0f34bf6daa198283
Summary: Fix the capitalization in the error message to match the other variants.
Reviewed By: singhsrb
Differential Revision: D22078125
fbshipit-source-id: 4a1de0111fa4dceea2d401c47ef145669a3cc6f5
Summary:
Rename the `subtree` endpoint on the EdenAPI server to `complete_trees` to better express what it does (namely, fetching complete trees, in contrast to the lighter weight `/trees` endpoint that serves individual tree nodes). This endpoint is not used by anything yet, so there isn't much risk in renaming it at this stage.
In addition to renaming the endpoint, the relevant request struct has been renamed to `CompleteTreeRequest` to better evoke its purpose, and the relevant client and test code has been updated accordingly. Notably, now that the API server is gone, we can remove the usage of this type from Mononoke's `hgproto` crate, thereby cleaning up our dependency graph a bit.
Reviewed By: krallin
Differential Revision: D22033356
fbshipit-source-id: 87bf6afbeb5e0054896a39577bf701f67a3edfec
Summary: Make most of the handlers on the EdenAPI server return streaming responses.
Reviewed By: krallin
Differential Revision: D22014652
fbshipit-source-id: 177e540e1372e7dfcba73c594614f0225da3a10f
Summary: The intention of this parameter was to produce human-readable JSON for tests and for manual debugging. In practice, given that the JSON response here is just a (typically short) list, there isn't really much value in prettifying. We weren't using this anyway, so let's remove it to simplify the code.
Reviewed By: xavierd
Differential Revision: D22020784
fbshipit-source-id: 09290cc667cfdb194693e6c05858af2f8905f88e
Summary: Add wrapper functions around `data::data` for files and trees. The intention is to make the code more easily searchable, and also to establish a consistent naming pattern across all the handler functions.
Reviewed By: xavierd
Differential Revision: D22020783
fbshipit-source-id: 5e7c63ff8f366b0c6aa4412d5d04d17ca6cbc6b9
Summary: Reduce the amount of repeated code in the handlers module by defining a macro to wrap async handler functions.
Reviewed By: krallin
Differential Revision: D22015557
fbshipit-source-id: 276dad4112e7019f8d5f5fa74243747a0104e1ca
Summary: Add an `errors` module containing an `ErrorKind` implemented via `thiserror`. All errors in the server's HTTP handles been annotated with an appropriate `ErrorKind` via `anyhow::Context`. Notably, there is specialized handling of `MononokeError`s from the `mononoke_api` crate via a new `MononokeErrorExt` trait.
Reviewed By: krallin
Differential Revision: D22012237
fbshipit-source-id: 5f6948696e4741816636f1c6fee12069ee1a4b32
Summary: Move the `util` module out of `handlers` and split it up into multiple submodules.
Reviewed By: krallin
Differential Revision: D22008148
fbshipit-source-id: 11b286fdef8f9d93fcc10a283ec1b2f42e9c8c04
Summary: Change `HistoryResponse` so that instead of repeatedly sending the file path alongside every history entry, we group the history entries by file and send the path once per group. This will prevent the server from transmitting redundant information.
Reviewed By: quark-zju
Differential Revision: D21982558
fbshipit-source-id: f1c5d2573c97940c7bf1645ed7fef6e1887c0d42
Summary: Last usage was removed in D20342689.
Reviewed By: xavierd
Differential Revision: D22011195
fbshipit-source-id: 58455ad3dc77438704e80c69eb728a319ee71a39
Summary: D21880220 renamed the `depth` parameter in Mononoke's history fetching functions to be `length`. This diff makes the same change for EdenAPI's `HistoryRequest` struct.
Reviewed By: StanislavGlebik
Differential Revision: D21948599
fbshipit-source-id: fe8649a5789f07d8262ad3d5e2f477a8b50f2c6f
Summary:
Add a new `subtree` endpoint to the EdenAPI server, which fetches trees using the underlying server-side logic for the `gettreepack` wire protocol command. This is intended for use in situations where compatibility with `gettreepack` is desired when using HTTP for tree fetching.
The name of the endpoint is up for bikeshedding. It seemed weird to name the endpoint `gettreepack` since that name is a verb and refers to a "pack" which is not relevant in this context (there are no wirepacks or packfiles involved). I chose the name `subtree` since the endpoint logically returns all of the nodes underneath a given node in the tree (though in the most frequent case, the node will be the root node and therefore the subtree will be the entire tree).
In practice, this initial implementation is not ideal because it buffers the trees in memory, which is problematic because `gettreepack` requests are likely to produce a very large number of trees. Later in this stack, the endpoint will be updated to produce a streaming response instead.
Reviewed By: StanislavGlebik
Differential Revision: D21782764
fbshipit-source-id: 726925858352c33c923da1979da9d20fbcf930f6
Summary:
Several of the structs used by EdenAPI were previously defined in Mercurial's
`types` crate. This is not ideal since these structs are used for data interchange
between the client and server, and changes to them can break Mononoke, Mercurial,
or both. As such, they are more fragile than other types and their use should be
limited to the EdenAPI server and client to avoid the need for extensive breaking
changes or costly refactors down the line.
I'm about to make a series of breaking changes to these types as part of the
migration to the new EdenAPI server, so this seemed like an ideal time to split
these types into their own crate.
Reviewed By: krallin
Differential Revision: D21857425
fbshipit-source-id: 82dedc4a2d597532e58072267d8a3368c3d5c9e7
Summary: The replacement will make lfs_server, mononoke_api, edenapi_server and walker OSS buildable.
Reviewed By: krallin
Differential Revision: D21884324
fbshipit-source-id: e6cdf8356b13056a9944ab9da18dc15977b6a2ec
Summary:
The max_depth parameter was confusing - one might have thought that it returns
all entries that are `max_depth` hops away from `startnode`. This is not the
case - in reality it just limits what's the total number of entries can be
returned.
In fact, I think it can be replaced with Stream::take() method, however that's
a larger refactoring, and I'm not going to do it now.
It doesn't matter much because usually we have linear histories, but I think
it's still worth to make it clearer.
Reviewed By: krallin
Differential Revision: D21880220
fbshipit-source-id: d209e1e129383f9181ae11b489992dc4c3ce2d5c
