- Jun 14, 2023
-
-
Georges Racinet authored
This time, because there is no arbitrary name to consider, hence no bytes string, Tokio's `LinesStream` is perfectly suitable for our needs.
-
Georges Racinet authored
This follows the file format and conventions defined in the server side `heptapod` Python package. Since everything is async, we actually provide Streams of TypedRef objects, and the `map_lookup_typed_ref` for the numerous use cases when one is looking for a specific ref. The file format is very primitive, so all we can do at this point is to perform a full scan, but it has room (version preamble) for more advanced formats in the future (e.g, append-only persistent hash map or similar). A preliminary version of this was using Tokio's `LineStream`, but this has several drawbacks: - it is unicode-centric, yielding `String`s - we had to implement our own `Stream`, keeping the `LineStream` as a field. Hence because `poll_next()` takes `&mut Pin<Self>` we had to write a pinning accessor, which is unsafe (see the doc for `std::pin` for more details).
-
Georges Racinet authored
When a file contains a list of values, it is a common pattern that the absence of the file is equivalent to the list being empty. This `io_error_not_found_as_none` will help keeping duplication low (even if fairly trivial).
-
- Jun 27, 2023
-
-
Georges Racinet authored
These are not frequent, hence systematic collecting is acceptable. On large repositories, it can prevent going over the RSS limit. Closes #132
-
- Jun 07, 2023
-
-
Georges Racinet authored
The case of the full node that does not exist goes through a quite different path in the sub-service layer implementation (will be moreso true with RHGitaly), it's worth its own test case.
-
Georges Racinet authored
-
- Jun 06, 2023
-
-
Georges Racinet authored
Previously, the test was passing because the Python reference implementation of HGitaly relies on `scmutil.revsingle` for revisions not found in GitLab state files, including tags given in simple form (e.g. `v1.2.3`). But RHGitaly will rely on the GitLab state files only, hence we need to ensure that the tags state file contains the one we introduce, which is shadowing in the short SHA case The added GitLab sync has the effect of removing directly set tags that have no Mercurial counterpart from the GitLab state file, that's why it has to occur before `write_ref()` in this test of collision/shadowing were Mercurial and Git tags must have different names.
-
Georges Racinet authored
As illustrated with the new assertions in `test_compare_find_commit`, it turns out that in Gitaly, tags have precedence over branches (unsurprisingly) and over shortened commit ids, but not on full commit ids.
-
- Jun 04, 2023
-
-
Georges Racinet authored
As usual, the thoroughness of Rust highlights some more pathological cases where the implementation could fail.
-
Georges Racinet authored
This method implementation probably predates the Comparison tests, hence the doubt left as comment, that can now be lifted. The new comparison test will be useful for #128: as usual, we'll just have to use the fixture with RHGitaly and parametrization. Also, it uncovers that if `ref` does not start with `refs/` the `INVALID_ARGUMENT` error is returned by Gitaly.
-
- May 26, 2023
-
-
Georges Racinet authored
This logs the whole blob data, typically a log introduced for development that should not have gone to production.
-
- May 16, 2023
-
-
Georges Racinet authored
-
Georges Racinet authored
A bit nicer in `test_ref` and will also be needed in `test_commit`
-
Georges Racinet authored
This already reduces duplication in `test_ref`, and will be reusable by other test modules.
-
Georges Racinet authored
The previous way of invalidating the Mercurial to Git map was to delete the cached property, but it raises an error if it has not been used before. In theory the invalidation could be removed if not necessary any more. In practice, this forces to check that it is normal. Better to encapsulate. Also, for simplicity, we also invalidate Mercurial caches by reloading the repo.
-
- May 15, 2023
-
-
Georges Racinet authored
There was even an old TODO about that
-
Georges Racinet authored
This method of the `CommitService` had no Gitaly Comparison test, because it was implemented before these tests were even considered. Such a test will be the main testing of the upcoming RHGitaly implementation. It turns out that we found a missing case: tag in the fully qualified ref notation (chances are that clients woud rather use `RefService.FindTag` but one never knows.
-
Georges Racinet authored
In some cases where a message attribute is missing, our normalization would have replaced it with a default (empty) value, leading to false positives. This is for instance the case of an empty `FindCommitResponse` (no commit attribute) that it would convert to a `FindCommitResponse` with an empy `commit` attribute (part of actual tests in the process of being written).
-
Georges Racinet authored
A separate method for each "attribute path" gives us a bit more clarity (early return if `recurse` rather than a boolean available from outside the inner loop), and more room for improvement.
-
- May 14, 2023
-
-
Georges Racinet authored
Recent Gitaly versions come with two versions of Git (probably to test them in advance), and hence the path that we were trying is no longer valid. It is now mandatory of to the development setup (HDK) and to the CI base image to provide Gitaly with the bundled Git. The concept of "bundled Git" seems to go beyond vendoring, the compilation flag `WITH_BUNDLED_GIT=YesPlease` has the effect of putting self-sufficient Git executables in Gitaly bin directory, which the main executable knows how to find and select, provided that the `bin_dir` configuration item is correct (fixed in a preparation changeset). Using this bundled Git removes lots of confusion and timebombs such as the tests defaulting to a system-wide Git which suddenly can become outdated.
-
Georges Racinet authored
Since we only test one candidate `gitaly` executable, we don't need the loop.
-
Georges Racinet authored
In case the `gitaly` executable is to be found in a subdirectory of `GITALY_INSTALL_DIR`, then that same subdirectory has to be provided as the `bin_dir` configuration item, so that Gitaly can find all its non-ruby bundled executables, notably the bundled Git. It was working until now because Gitaly falls back to a system-wide Git if the specified one is not found, whose version is acceptable in CI because the `heptapod-gitaly` base image is itself based on the `heptapod-base` image, which provides the Git version generally accepted by the target GitLab version. Since there is no doubt as to where the `gitaly` executable is supposed to sit, we're only testing one possibility (minimal change to keep the diff understable, to be refactored).
-
- May 03, 2023
-
-
Georges Racinet authored
-
- May 01, 2023
-
-
Georges Racinet authored
Hoping this will reuse intermediate object files from the artifact uploaded by the `tests-rust` job
-
- Apr 30, 2023
-
-
Georges Racinet authored
Again, we don't want the application to be locked for the wrong reason, e.g. lock held by a former process, but hostname has changed in the mean time. We also take the opportunity to use shorter timeout and delay betweeni locking attempts. The customatbility of the breaking logic and of the timeouts could be submitted UPSTREAM as options to Mercurial core, although it would be of limited interest to Heptapod right now: we cannot readily apply the same logic to other Mercurial locks, because HGitaly does not have exclusivity on them. Querying the system service manager (systemd or runit) about the Heptapod services would be a promising idea to do better, but we'd still have the issue that pushes over SSH would create standalone processes.
-
- Apr 26, 2023
-
-
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.
-
- Apr 25, 2023
-
-
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.
-
Georges Racinet authored
This makes it importable from the whole code base, and is already somewhat cleaner in the `tests_with_gitaly.rhgitaly` module.
-
- Apr 24, 2023
-
-
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.
-
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.
-
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.
-
- Apr 09, 2023
-
-
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.
-
- Apr 28, 2023
-
-
Georges Racinet authored
-
- Apr 27, 2023
-
-
Georges Racinet authored
-
Georges Racinet authored
Worth of note is that the log formatting will not be applied unless the level if fixed low enough. That's why the tests did not detect the problem in CreateBundleFromRefList, But it is visible in the production log
-
- Apr 26, 2023
-
-
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.
-
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.
-
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).
-
- Apr 25, 2023
-
-
Georges Racinet authored
We are now on hg-evolve 11. This is the promised backout of 1c187a5a5a0c, with conflict resolution.
-