Skip to content
Snippets Groups Projects
  1. Apr 26, 2023
    • Georges Racinet's avatar
      workdir: auto-release if held by an invalid process id · fb44e1f5
      Georges Racinet authored
      The rationale is that if a working directory is being held by a process (existing
      or not) that is not considered part of the HGitaly system service, it means that
      the process was killed before it could release the working directory and the pid
      is either not in use or has been reused for an unrelated process. The underlying
      assumption is that only HGitaly is supposed to make such working directories.
      
      This could be further expanded to the roster lock.
      
      Had to put `is_current_service_process` in a separate top level module
      instead of, e.g., `server.mono` to avoid import loop:
        server.mono imports services import workdir import server.mono
      
      We need to use pytest-cov's `cleanup_on_signal` again. This time,
      it's easier as the subprocess startup function is defined by the test itself.
      fb44e1f5
  2. Apr 25, 2023
    • Georges Racinet's avatar
      server.mono: covering the startup function · 57761166
      Georges Racinet authored
      The trick is that we fall precisely in the case where pytest-cov
      needs a hand: termination by signal (SIGTERM), hence we need the
      `cleanup_on_signal` method.
      
      Unfortunately, this does not work with pytest-7.3.1 and pytest-cov-4.0.0
      as the previously lax `test-requirements.txt` would install (detected
      on the `heptapod-gitaly` base image, which does not preinstall these),
      so we fix versions as they are in py-heptapod (hence also as they already
      were in HDK settups, since `py-heptapod` is installed first).
      
      Then, we need the server to actually be running to be sure that
      the handling has been done before we terminate.
      
      Still not covering the case of `SIGINT` while doing some
      Mercurial processing. We'd probably need to launch a long enough
      gRPC method for that.
      57761166
    • Georges Racinet's avatar
      Comparison tests: moved server full startup wait function to hgitaly pkg · 86e2b479
      Georges Racinet authored
      This makes it importable from the whole code base, and is already
      somewhat cleaner in the `tests_with_gitaly.rhgitaly` module.
      86e2b479
  3. Apr 24, 2023
    • Georges Racinet's avatar
      servicer: expose context manager for working directories on self · 15c56172
      Georges Racinet authored
      This wraps things together by passing down the proper root of working
      directories for the given repo (as prescribed in the Gitaly `Repository`
      message.
      
      The test is a simplified version of a service method creating changesets.
      15c56172
    • Georges Racinet's avatar
      servicer: split out relative path normalization · dc2defec
      Georges Racinet authored
      It has already been the case in Heptapod that relative path in
      Gitaly Repository messages were to be passed as if the repository
      was a Git repository (hence ending in `.git`) and then to be
      normalized as `.hg` for Mercurial operation. But this is due
      to the legacy hg-git based integration, and could not be necessary
      in some future.
      
      Meanwhile, we'll need to reuse the logic for working directories.
      dc2defec
    • Georges Racinet's avatar
      working directories: garbage collection · 081e8130
      Georges Racinet authored
      This is to be called as part of ordinary housekeeping.
      
      Another subject to be tackled on would be release of
      working directories held by defunct processes, which would
      introduce a more satisfactory breaking logic, also applicable
      to the roster lock.
      081e8130
  4. Apr 09, 2023
    • Georges Racinet's avatar
      working directories: inner pool handling · b3e7a172
      Georges Racinet authored
      This creates the pool, recorded as a "roster" file
      in the main repository.
      
      To obtain an exclusive working directory, callers should
      use the `working_directory` context manager only.
      
      The roster file is protected by a lock, with input/output
      occurring only at acquire and release times of working
      directories, allowing to acquire working directories freely
      when one is already held. The lock is for now provided by
      Mercurial standard utilities, but we will probably implement
      a more suitable one later on.
      
      A cleanup system for old working directories shall be implemented
      in a following changeset. It will be based on the recorded timestamp.
      b3e7a172
  5. Apr 28, 2023
  6. Apr 27, 2023
  7. Apr 26, 2023
    • Georges Racinet's avatar
      branch: avoid fallback analysis on empty repos · 141dab17
      Georges Racinet authored
      We've been worried to see so many "defaulting to slow direct
      analysis" in the logs (especially in developer setups), as we
      are on the verge to rely on the GitLab state files only in some
      cases (RHGitaly).
      
      It turns out that all such logs emitted by a local functional tests
      run were actually about empty repos
      (`heptapod-tests-run --heptapod-remote --heptapod-hg-native=full`).
      It will be interesting to check foss.heptapod.net logs. In theory, we
      should not see any such warning.
      
      The early returns are of course much simpler, not sure if that
      is a performance boost in general, though, since the condition will be
      checked for all calls and most of them are not on empty repositories.
      141dab17
    • Georges Racinet's avatar
      RHGitaly: avoid logging all metadata · 32b0304d
      Georges Racinet authored
      Logging all invocation metadata is verbose and contains secrets
      (authentication header). Of these, only the `correlation_id` is
      interesting to us at the moment.
      
      To achieve this, we split the service method with an inner method
      taking the inner request and the correlation id. Only the inner
      method is now instrumented.
      
      This is heavy for the implementation.
      We tried not to split, using the `fields` modifier like this:
      
      ```
          #[instrument(skip(self, request), fields(request = request.get_ref()))]
      ```
      
      and it ended up giving "`RepositoryRequest` does not implement tracing::Value`"
      
      The splitting will be good enough for now.
      32b0304d
    • Georges Racinet's avatar
      Gitaly Comparision test_tree_entry_requests: giving up on comparing chunk size · dee21e96
      Georges Racinet authored
      Basically, Gitaly chunk lengths are partly unpredictable. We tried to approximate
      them, but we can't really depend on them in assertions.
      
      The irritating thing is that this test failed often in CI (clear majority of runs,
      yet not all of them) and passed almost always on my development setup. The CI
      flakiness is now too much of a pain to keep it.
      
      Passing the `compare_first_chunks=False` does not change the fact that the
      entire content is compared and that metadata (aka small fields or unchunked fiels)
      are compared for all chunks (added a clarification in docstring about that).
      dee21e96
  8. Apr 25, 2023
  9. Apr 24, 2023
  10. Apr 13, 2023
  11. Apr 12, 2023
  12. Mar 24, 2023
  13. Apr 02, 2023
    • Georges Racinet's avatar
      CI/CD: Rust lints (rustfmt, clippy and line widths) · f4e5c429
      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.
      f4e5c429
    • Georges Racinet's avatar
      RHGitaly: fixed Clippy warnings · 555e544f
      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
      555e544f
  14. Mar 26, 2023
  15. Mar 23, 2023
    • Georges Racinet's avatar
      RHGitaly Comparison tests: comparison with HGitaly for ServerInfo · c7d863a5
      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.
      c7d863a5
    • Georges Racinet's avatar
      Comparison tests: cut off BaseRpcHelper from RpcHelper · 42fb2ee5
      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.
      42fb2ee5
    • Georges Racinet's avatar
      RHGitaly: proper version information · bd9e9d77
      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.
      bd9e9d77
    • Georges Racinet's avatar
      CI/CD RHGitaly: using artifacts and cache · 9be2e5b3
      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.
      9be2e5b3
    • Georges Racinet's avatar
      CI/CD: running pure Rust tests · 862e93be
      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).
      862e93be
  16. Mar 19, 2023
    • Georges Racinet's avatar
      RHGitaly: Gitaly comparison tests · 69cb7195
      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
      69cb7195
Loading