Summary:
502 made a bit of sense since we can occasionally proxy things to upstream, but
it's not very meaningful because our inability to service a batch request is
never fully upstream's fault (it would not a failure if we had everything
internally).
So, let's just return a 500, which makes more sense.
Reviewed By: farnz
Differential Revision: D20897250
fbshipit-source-id: 239c776d04d2235c95e0fc0c395550f9c67e1f6a
Summary:
I noticed this while doing some unrelated work on this code. Basically, if we
get an error from upstream, then we shouldn't return an error the client
*unless* upstream being down means we are unable to satisfy their request
(meaning, we are unable to say whether a particular piece of content is
definitely present or definitely missing).
This diff fixes that. Instead of checking for a success when hearing form
upstream _then_ running our routing logic, let's instead only fail if in the
course of trying to route the client, we discover that we need a URL from
upstream AND upstream has failed.
Concretely, this means that if upstream blew up but internal has all the data
we want, we ignore the fact that upstream is down. In practice, internal is
usually very fast (because it's typically all locally-cached) so this is
unlikely to really occur in real life, but it's still a good idea to account
for this failure scenario.
Reviewed By: HarveyHunt
Differential Revision: D20897252
fbshipit-source-id: f5a8598e8a9da382d0d7fa6ea6a61c2eee8ae44c
Summary:
D20139308 added a FromStr implementation for Identity. The LFS
server uses the same code when reading identities from the command line.
Update `idents_from_values` to use the `FromStr` implementation.
Reviewed By: krallin
Differential Revision: D20441439
fbshipit-source-id: 5809ed54d1971fd778b82cc726739a382312747a
Summary: Since they are static and immutable, fbwhoami/fbwhatami could be simple structs with public fields.
Reviewed By: eugeneoden
Differential Revision: D20299423
fbshipit-source-id: 492f49c2b3003760517bfc5be06ace07fabbc6b9
Summary:
Our post-request processing was still implemented using old futures. Let's get
rid of that, and make it all new futures. This will make it easier to maintain,
and means we can get rid of the tokio-old dependency.
Reviewed By: mitrandir77
Differential Revision: D20343188
fbshipit-source-id: 513e124805e2ca588a1e312d8f5ca697ed6030c8
Summary:
This updates the lfs server and eden api server to use a newer version of
Gotham, which comes along with an updated version of Bytes and Hyper.
A few things had to change for this:
- New bytes don't support concatenation, so we need to fold them ourselves,
except...
- ... new Hyper bodies don't tell you how big they are (either in requests or
responses), so we need to inspect headers to find the size instead (I added
this in `gotham_ext::body_ext::BodyExt`, although it arguably belongs more in
a `hyper_ext` crate, but creating a new crate for just this seems overkill).
- New Hyper requires its data stream to be `Sync` for reasons that have more to
do with developer experience than runtime
(https://github.com/hyperium/hyper/pull/1857). Unfortunately, our Filestore
streams aren't `Sync`, because our `BoxFuture` contains a `dyn Future` that
isn't explicitly `Sync` (which is how we pull things out of blobstores). Even
if `BoxFuture` contained a `Sync` future, that still wouldn't be enough
anyway, because `compat()` explicitly implements `!Sync` on the stream it
returns. I'll ask upstream in Hyper if this can possibly change in the
future, but for now we can work around it by wrapping the stream in a
channel. I'll keep an eye out for performance here.
- When I updated our "pre state data" tweaks on top of Gotham, I renamed those
to "socket data", since that's a better name or what they are (hence the
changes here).
- I updated the lfs_protocol to stop depending on Hyper and instead depend on
http, since that's all we need here.
As you review this, please pay close attention to the updated implementation of
`SignalStream`. Since this is a custom `Stream` in new futures, it requires a
bit of `unsafe { ... }`.
Note that, unfortunately, the diff includes both the Gotham update and the
server updates, since they have to happen together.
Reviewed By: kulshrax, dtolnay
Differential Revision: D20342689
fbshipit-source-id: a490db96ca7c4da8ff761cb80c1e7e3c836bad87
Summary:
A lot of callsites want to know repo name. Currently they need to pass it from
the place where repo was initialized, and that's quite awkward, and in some
places even impossible (i.e. in derived data, where I want to log reponame).
This diff adds reponame in BlobRepo
Reviewed By: krallin
Differential Revision: D20363065
fbshipit-source-id: 5e2eb611fb9d58f8f78638574fdcb32234e5ca0d
Summary:
The goal of the whole stack is quite simple (add reponame field to BlobRepo), but
this stack also tries to make it easier to initialize BlobRepo.
To do that BlobrepoBuilder was added. It now accepts RepoConfig instead of 6
different fields from RepoConfig - that makes it easier to pass a field from
config into BlobRepo. It also allows to customize BlobRepo. Currently it's used
just to add redaction override, but later we can extend it for other use cases
as well, with the hope that we'll be able to remove a bunch of repo-creation
functions from cmdlib.
Because of BlobrepoBuilder we no longer need open_blobrepo function. Later we
might consider removing open_blobrepo_given_datasources as well.
Note that this diff *adds* a few new clones. I don't consider it being a big
problem, though I'm curious to hear your thoughts folks.
Note that another option for the implementation would be to take a reference to objects
instead of taking them by value. I briefly looked into how they used, and lot of them are passed to the
objects that actually take ownership of what's inside these config fields. I.e. Blobstore essentially takes ownership
of BlobstoreOptions, because it needs to store manifold bucket name.
Same for scuba_censored_table, filestore_params, bookmarks_cache_ttl etc. So unless I'm missing anything, we can
either pass them as reference and then we'll have to copy them, or we can
just pass a value from BlobrepoBuilder directly.
Reviewed By: krallin
Differential Revision: D20312567
fbshipit-source-id: 14634f5e14f103b110482557254f084da1c725e1
Summary:
Note that comparing to many other asyncifying efforts, this one actually adds
one more clone instead of removing them. This is the clone of a logger field.
That shouldn't matter much because it can be cleaned up later and because this
function will be called once per repo.
Reviewed By: krallin
Differential Revision: D20311122
fbshipit-source-id: ace2a108790b1423f8525d08bdea9dc3a2e3c37c
Summary:
Context: https://fb.workplace.com/groups/rust.language/permalink/3338940432821215/
This codemod replaces all dependencies on `//common/rust/renamed:tokio-preview` with `fbsource//third-party/rust:tokio-preview` and their uses in Rust code from `tokio_preview::` to `tokio::`.
This does not introduce any collisions with `tokio::` meaning 0.1 tokio because D20235404 previously renamed all of those to `tokio_old::` in crates that depend on both 0.1 and 0.2 tokio.
This is the tokio version of what D20213432 did for futures.
Codemod performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs, rdeps(%Ss, //common/rust/renamed:tokio-preview, 1))" \
| xargs sed -i 's,\btokio_preview::,tokio::,'
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| xargs sed -i 's,//common/rust/renamed:tokio-preview,fbsource//third-party/rust:tokio-preview,'
```
Reviewed By: k21
Differential Revision: D20236557
fbshipit-source-id: 15068b93a0a944d6249a1d9f63840a4c61c9c1ba
Summary:
Context: https://fb.workplace.com/groups/rust.language/permalink/3338940432821215/
In targets that depend on both 0.1 and 0.2 tokio, this codemod renames the 0.1 dependency to be exposed as tokio_old::. This is in preparation for flipping the 0.2 dependencies from tokio_preview:: to plain tokio::.
This is the tokio version of what D20168958 did for futures.
Codemod performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs,
rdeps(%Ss, fbsource//third-party/rust:tokio-old, 1)
intersect
rdeps(%Ss, //common/rust/renamed:tokio-preview, 1)
)" \
| xargs sed -i 's,\btokio::,tokio_old::,'
```
Reviewed By: k21
Differential Revision: D20235404
fbshipit-source-id: cfb2689a584ad0d73f16d98d8587fb9c44661465
Summary:
Context: https://fb.workplace.com/groups/rust.language/permalink/3338940432821215/
This codemod replaces *all* dependencies on `//common/rust/renamed:futures-preview` with `fbsource//third-party/rust:futures-preview` and their uses in Rust code from `futures_preview::` to `futures::`.
This does not introduce any collisions with `futures::` meaning 0.1 futures because D20168958 previously renamed all of those to `futures_old::` in crates that depend on *both* 0.1 and 0.3 futures.
Codemod performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs, rdeps(%Ss, //common/rust/renamed:futures-preview, 1))" \
| xargs sed -i 's,\bfutures_preview::,futures::,'
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| xargs sed -i 's,//common/rust/renamed:futures-preview,fbsource//third-party/rust:futures-preview,'
```
Reviewed By: k21
Differential Revision: D20213432
fbshipit-source-id: 07ee643d350c5817cda1f43684d55084f8ac68a6
Summary:
* Added intermediate (de)serializers for config types, so that we generate full Identity objects at config load time
* Implement FromStr for Identity
* Compare configured identities to presented identities in ratelimit middleware in order to decide whether or not to apply the limit
Reviewed By: krallin
Differential Revision: D20139308
fbshipit-source-id: 340c300db549575eb6d06efcbe437c0b1db4927b
Summary:
In targets that depend on *both* 0.1 and 0.3 futures, this codemod renames the 0.1 dependency to be exposed as futures_old::. This is in preparation for flipping the 0.3 dependencies from futures_preview:: to plain futures::.
rs changes performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs,
rdeps(%Ss, fbsource//third-party/rust:futures-old, 1)
intersect
rdeps(%Ss, //common/rust/renamed:futures-preview, 1)
)" \
| xargs sed -i 's/\bfutures::/futures_old::/'
```
Reviewed By: jsgf
Differential Revision: D20168958
fbshipit-source-id: d2c099f9170c427e542975bc22fd96138a7725b0
Summary:
This updates our middleware stack and introduces two new pieces of functinality:
- Middleware can now be async.
- Middleware can now preempt requests and dispatch a response.
The underlying motivation for this is to allow implementing Mononoke LFS's rate
limiting middleware in our existing middleware stack.
Reviewed By: kulshrax
Differential Revision: D20191213
fbshipit-source-id: fc1df7a14eb0bbefd965e32c1fca5557124076b5
Summary:
The Bytes 0.5 update left us in a somewhat undesirable position where every
access to our blobstore incurs an extra copy whenever we fetch data out of our
cache (by turning it from Bytes 0.5 into Bytes 0.4) — we also have quite a few
place where we convert in one direction then immediately into the other.
Internally, we can start using Bytes 0.5 now. For example, this is useful when
pulling data out of our blobstore and deserializing as Thrift (or conversely,
when serializing and putting it into our blobstore).
However, when we interface with Tokio (i.e. decoders & encoders), we still have
to use Bytes 0.4. So, when needed, we convert our Bytes 0.5 to 0.4 there.
The tradeoff idea is that we deal with more bytes internally than we end up
sending to clients, so doing the Bytes conversion closer to the point of
sending data to clients means less copies.
We can also start removing those once we migrate to Tokio 0.2 (and newer
versions of Hyper for HTTP services).
Changes that were required:
- You can't extend new bytes (because that implicitly copies). You need to use
BytesMut instead, which I did where that was necessary (I also added calls in
the Filestore to do that efficiently).
- You can't create bytes from a `&'a [u8]`, unless `'a` is `'static`. You need
to use `copy_from_slice` instead.
- `slice_to` and `slice_from` have been replaced by a `slice()` function that
takes ranges.
Reviewed By: StanislavGlebik
Differential Revision: D20121350
fbshipit-source-id: eb31af2051fd8c9d31c69b502e2f6f1ce2190cb1
Summary:
Mercurial wishes to use this crate, but pulling in mononoke_types brings way
too many dependencies. Since the only reason mononoke_types is brought in is
for the Sha256 type, let's just hardcode it to [u8; 32].
Reviewed By: krallin
Differential Revision: D20003596
fbshipit-source-id: 53434143c61cd1a1275027200e1149040d30beae
Summary: Add the max_jitter_ms field to the rate limiting config struct, and to the integration test.
Reviewed By: HarveyHunt
Differential Revision: D19905068
fbshipit-source-id: b44251c456a45bc494d1080e405f2d009becc0d2
Summary: Move several pieces of Middleware related to TLS from the LFS server to `gotham_ext`, for reuse in the EdenAPI server. These modules are already well-abstracted and consequently require no modification to be moved out of the LFS server.
Reviewed By: xavierd
Differential Revision: D19890537
fbshipit-source-id: 8de420183e7bd5dd0de10a75c8cfe83825f0c23c
Summary: Move the generally-useful ServerIdentityMiddleware into gotham_ext so we can use it in the EdenAPI server.
Reviewed By: xavierd
Differential Revision: D19845282
fbshipit-source-id: 3a01b15dc64cee99cefafcdac229c0b70a4db683
Summary:
Added "rust" to the languages in the if-service and its dependent targets, added a unique `rust_crate_name` to each one.
Adjusted import names in the dependent code.
Reviewed By: dtolnay
Differential Revision: D19818989
fbshipit-source-id: d1cf5c5f7b82dd64d7c045e50b304b7a58cf2ffc
Summary:
This commit manually synchronizes the internal move of
fbcode/scm/mononoke under fbcode/eden/mononoke which couldn't be
performed by ShipIt automatically.
Reviewed By: StanislavGlebik
Differential Revision: D19722832
fbshipit-source-id: 52fbc8bc42a8940b39872dfb8b00ce9c0f6b0800
Summary:
See D19787960 for more details why we need to do it.
This diff just adds a struct in BlobRepo
Reviewed By: HarveyHunt
Differential Revision: D19788395
fbshipit-source-id: d609638432db3061f17aaa6272315f0c2efe9328
