- Apr 24, 2023
-
-
Georges Racinet authored
-
- Apr 13, 2023
-
-
Georges Racinet authored
-
Georges Racinet authored
Closes #118
-
Georges Racinet authored
This parallels the Python implementation: returning a `GitCommit`, this time from a `ChangelogEntry` instead of Python `changectx` will be a common need. The user line parsing is implemented to match the Python implementation (module `stringutil`). We considered using a regular expression, but it is not so obvious to obtain the exact same result (actually first experiments where splitting the brackets with a regexp for quick validation of the first gRPC method that will use this). User line and timestamp parsing should find eventually their way into `hg-core`.
-
Georges Racinet authored
All future implementations of streaming methods with repositories will follow the same pattern, hence we provide a generic helper right away, taking care notably of repetitive error treatment (sending errors etc.). Detailed explanations of the constraints and how it will work are provided in the doc-comment of the `load_repo_and_stream` function. Concrete gRPC method implementations will consist mostly of calling it with the closure doing the actual work on the loaded repository.
-
Georges Racinet authored
As the Gitaly Comparison tests showed us in the Python case, the errors to return on wrong `Repository` specifications depend at least on the service, hence we need an added flexibility. For that, we need internally to make a difference between client-side errors (e.g., repo does not exist) and internal errors (failed to load repo, e.g., due to a filesystem issue).
-
- Apr 12, 2023
-
-
Georges Racinet authored
That no response at all should be sent is confirmed by the added assertions in the Gitaly Comparison test
-
- Mar 24, 2023
-
-
Georges Racinet authored
Basic, yet will provide the baseline for RHGitaly development.
-
Georges Racinet authored
not immediately needed, but was overlooked
-
Georges Racinet authored
Reflection with `isinstance` is good enough, but for the sake of symmetry with responses we should perhaps add brackets to the specification, as in `request_sha_attrs='oids[]`
-
- Apr 02, 2023
-
-
Georges Racinet authored
As we don't want to resort ot unstable features of rustfmt, we wrote a simple checker in Python. Also we have to run after the tests, because we need the protocol source files (e.g., `gitaly.rs` to be present). Without them, `cargo fmt` fails at the `use` statement.
-
Georges Racinet authored
Some warnings have been fixed lower in the stack. However, the Camel Case rule for acronyms hurts my eyes in the case of TCP (and leads to plain wrong output when it ends up in startup logs through `fmt::Debug`). We had to change the way the generated code is included. This also has the nice effect of ignoring `gitaly.rs` in `rustfmt`, thus making `cargo fmt --check` work for us. The missing `Eq` impl when deriving `PartialEq` is new in Rust 1.63
-
- Mar 26, 2023
-
-
Georges Racinet authored
Nicer when starting from scratch: we avoid a timeout error. It's still worthwile to have CI do it before hand, because the build log ends up in the job log.
-
- Mar 23, 2023
-
-
Georges Racinet authored
We're defining a new fixture and `RpcHelper` for such comparisons. As expected, this is much simpler than the one for Gitaly Comparison tests. The included test is actually useful: we are sure that we don't have version skew between RHGitaly and HGitaly. Given how it is implemented, this is not a surprise, but it'll have our back in case the implementation has to change.
-
Georges Racinet authored
This new base class does not carry the specificities of the Gitaly comparison, but still has feature flags, default args, and calling primitives. It will be useful to introduce Comparison tests between HGitaly and RHGitaly with room for growth.
-
Georges Racinet authored
It was nice to use the purely stateless `ServerInfo` gRPC method (at least with the info that HGitaly provides) as a first example of an implementation using Tonic. Now it's time to provide the actual information, consistent with the Python implementation. We're using the `build_const` crate to inject the value at build time. This is actually importing from the `constants.rs` file in the relevant build `out/` directory. Also the `..Default::default()` syntax (taught us by clippy) allows to avoid starting from the defaults to mutate a few fields.
-
Georges Racinet authored
This artifacts built by `tests-rust` will serve as precompilation for gitaly-comparison. For the cache, we follow the instructions of https://doc.rust-lang.org/nightly/cargo/guide/cargo-home.html#caching-the-cargo-home-in-ci but still exclude installed binaries information Hopefully, this will speed up the build a bit.
-
Georges Racinet authored
A simple `cargo test`. Because of the dependencies, we had to provide a new base image for this (we could have used heptapod-gitaly, but it is unsatisfactory to rely on a huge image when much less is needed).
-
- Mar 19, 2023
-
-
Georges Racinet authored
We chose to compare RHGitaly with Gitaly rather than with HGitaly because it's not impossible that we stop at some point maintaining some methods in the Python code (time will tell). It is likely that we'll end up implementing some of the HGitaly-specific methods in RHGitaly (e.g. reading phases, topics and whatnot). In that case we'll need comparison between HGitaly and RHGitaly, but that will be some much simpler (no correspondence of Git content) that we will provide it directly. This fully hooked in CI/CD. We had to anyway: otherwise the new test would not pass. Implementation details ---------------------- We add the capability to start RHGitaly, with a simple `cargo run`. This is validated independently by `test_rhgitaly_server` (homologous to `test_gitaly_server`). The `GitalyComparison` class (of which the `gitaly_comparison` fixture is an instance) now gains an optional `rhgitaly_channel`, which is set and exposed in the new `gitaly_rhgitaly_comparison` fixture. Finally, using `rhgitaly_channel` rather than `hgitaly_channel` is left to the `RPCHelper` class, with its `hg_server` argument. Since it is instantiated from inside the tests, it is easy to pass `hg_server` down from a parametrization, reaching the end goal: the very same test is run for HGitaly and RHGitaly, compared in both cases with Gitaly. This is illustrated in the test for `RepositoryExists`. Two annoyances had to be taken care of: - we need to ignore the `rust/` directory in `run-all-tests` because it makes pytest collection crazy (hangs forever) due at least in part to Mercurial full source being present by default in `rust/dependencies` - Tonic seems to add trailing metadata that Gitaly does not. We had to remove them in `assert_compare_errors()`. CI/CD assumptions ----------------- The base image `heptapod-gitaly` is now expected to provide - the Rust toolchain - a clone of Mercurial sources. It can be daily or weekly, we make a share and update it - `rust-protobuf`: could be trimmed, as we only need the protocol files and don't need to update them for each build
-
- Mar 18, 2023
-
-
Georges Racinet authored
We keep on configuring through environment variables, in order to focus on the task at hand: stop hardoding a TCP address and support Unix Domain sockets. The latter are the natural choice for integration tests, such as the Gitaly Comparison tests, as they come with strong guarantees not to be already in use.
-
- Mar 17, 2023
-
-
Georges Racinet authored
Similarly to the Python implementation, this introduces a top level module to take care of logic that will be useful for several methods. These top-level modules don't know about actual method or services, but they handle protobuf messages, `Status` errors etc. Like the Python implementation, we support only one storage (named `default`) and configuration does its repositories root.
-
- Mar 28, 2023
-
-
Georges Racinet authored
This removes the (blocking) `println!` in favour of the standard `tracing` crate, which is precisely meant for asynchronous code. For now, we're exclusively logging to `stdout`, at the `INFO` level. Configurability will appear later. The `DEBUG` level is very verbose, as it includes lots of events from the tonic/h2 etc lower layers. These will certainly be interesting, yet not at this stage of developpement. Quoting from current documentation: The #[instrument] attribute provides an easy way to add tracing spans to functions. A function annotated with #[instrument] will create and enter a span with that function’s name every time the function is called, with arguments to that function will be recorded as fields using `fmt::Debug`. Hence we don't need to include the passed `request` in the call to `info!`. This also raises the question of the laziness of formatting, something that we will have to check later. Finally, Tonic's `Request` wrapper does include all invocation metadata, which is great to have `correlation_id` but won't be acceptable when we implement authentication, as it will also probably have the related secrets. It could be a simple matter of using an appropriate subscriber. We'll see.
-
- Mar 17, 2023
-
-
Georges Racinet authored
We implement the `ServerInfo` gRPC. This is still very much a PoC: hardcoded address, no config or any kind of shared state, no proper (async) logging and the `server_info` value is not even valid (yet helpful to distinguish with other attempts on the same workstation). It is a of course unfortunate for an example that the simplest service that we can implement is `ServerService`. This leads us to niceties such as `ServerServiceServer`. Compare to the HelloWorld tutorial where the service is called `Greeter` and leads to a `GreaterServer` module. We decided to provide the `server_server` factory function to keep imports from generated code out of `main.rs`. Subsequent ones will be clearer: `commit_server` etc.
-
- Mar 18, 2023
-
-
Georges Racinet authored
-
- Mar 17, 2023
-
-
Georges Racinet authored
The idea is that we'll maintain a full clone of the Mercurial sources as `rust/dependencies/mercurial`, and the symlink will take the `hg-core` crate and anchor it to the workspace for RHGitaly, instead of the one from Mercurial sources. Because of this, we need to exclude the `rust` directory in `run-all-tests`. Otherwise, it makes pytest collection go crazy and hang forever. Also, we're versioning `Cargo.lock` because `rhgitaly` is a binary crate.
-
- Apr 12, 2023
-
-
Georges Racinet authored
Part of #80. As noticed by the functional tests at heptapod-tests, this is mandatory for Wikis using regular gRPC calls, which are rolled out in GitLab 15.5
-
- Mar 31, 2023
-
-
Georges Racinet authored
-
Georges Racinet authored
-
Georges Racinet authored
The only change we had to make is a workaround for what appears to be a minor bug in Mercurial or, if it is intended that some cache has to be updated explicitely, a missing compatibility in `mercurial-testhelper`.
-
Georges Racinet authored
This is in advance of !140 (introduction of RHGitaly), but is harmless if the directory does not exist and removes an irritating pain when switching topics.
-
Georges Racinet authored
py-heptapod 4.0 requires hg-evolve<11 and hence also Mercurial<6.4
-
- Mar 23, 2023
-
-
Georges Racinet authored
It turns out that `CommitService` does not return the `INTERNAL` gRPC error any more in Gitaly 15.5 when storage is incorrect. Now that is `INVALID_ARGUMENT`, same as with other gRPC methods.
-
Georges Racinet authored
The tests for `TreeEntry` and `GetTreeEntries` predate the introduction of the `RpcHelper` class by a few months, and were probably important in defining what we wanted to mutualize. Switching these tests to `RpcHelper` makes them directly applicable to `RHGitaly` when the time comes. The Blob/Tree methods are natural candidates for oxidation, as it is obvious that they are too slow in the Python implementation. We had to overcome two challenges for this: - Gitaly chunking being a bit non-deterministic in `TreeEntry`, we needed a way to compare streaming requests without comparing response by response. This now done by `RpcHelper.assert_compare_aggregated`, which takes care of the small data fields (`oid` and `mode` in the case of `TreeEntry`) with the common Gitaly pattern of providing them in the first response only. Two new hooks are to be registered in `RpcHelper` for this: one for the concatenation, one to remove the main data so that the small fields can be compared. - To take pagination into account, we needed a way to expose the first response of each backend as they were before normalization removed the cursor information (which is backend-specific). Since the pagination subprotocol is meant to be used in several streaming gRPC methods, we'll probably provide this in `RpcHelper` itself in a second move. In the process we're doing a more thorough job of comparing the `oid` response fields, thanks to a normalizer that knows the mapping between HGitaly and Gitaly OIDs.
-
- Mar 22, 2023
-
-
Georges Racinet authored
We had a Gitaly Comparison test, but it was actually comparing HGitaly with itself (!), see the change in `assert_compare_tree_entry()`. Then of course, whether it was initially correct or not, we had discrepancies to fix: - a gRPC `NOT_FOUND` is expected for unknown paths and revisions - `.` is not a valid path *for this method* - chunking in Gitaly happens in chunks of 16 kiB, but it's not consistent: sometimes the last chunk can be up to 16 kiB + 1023B long, sometimes, we get exact chunks. It's not clear in Gitaly code base exactly what happens: added logs in `streamio.go` showed us that this is what gets passed to it (not recut, then, since it's meant to cut at 128kiB). For these reasons, we gave up on comparing chunk by chunk and test the metadata only.
-
Georges Racinet authored
Since we don't currently run tests for the `stable` and `default` Mercurial/Evolve revisions (see 1c187a5a5a0c), we are at risk of losing coverage without the Gitaly Comparison tests. Hence we introduce the `tests-hg-current` job. Because the `gitaly-comparison` job (new name) will become heavier with the introduction of RHGitaly, we're pushing it to the compat stage. We could also just run `tests_with_gitaly` and don't bother with coverage (since it is done in a harder way by `tests-hg-current`).
-
Georges Racinet authored
-
Georges Racinet authored
Turns out Gitaly issues an `INTERNAL` error in that case. Probably not intended, but we're ready for further changes down the road.
-
Georges Racinet authored
It passes right away in that case.
-
Georges Racinet authored
As it was uncovered by experimental implementation in RHGitaly of `RepositoryExists`, it's possible that the `repository` argument is entirely missing. In that case, we are now raising `ValueError` internally, and convert it to the gRPC code `HGitalyServicer.STATUS_CODE_STORAGE_NOT_FOUND`.
-