Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hgweb race condition in GetLastCommitTime #1026

Merged
merged 14 commits into from
Aug 20, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Aug 14, 2024

If a project has just been created and then a zip file uploaded to populate it with initial data, as happens often in integration tests, then hgweb could return 404 for /hg/projectCode/log because its internal list of repos was recently refreshed (by another test) and the refresh interval hasn't been reached yet. One way to avoid this is to use the command runner to get the last commit date, since the hg command line doesn't use the refreshinterval setting from hgweb.

Fixes #1025.

If a project has just been created and then a zip file uploaded to
populate it with initial data, as happens often in integration tests,
then hgweb could return 404 for /hg/projectCode/log because its internal
list of repos was recently refreshed (by another test) and the refresh
interval hasn't been reached yet. One way to avoid this is to use the
command runner to get the last commit date, since the hg command line
doesn't use the refreshinterval setting from hgweb.
@rmunn rmunn requested a review from hahn-kev August 14, 2024 02:53
@rmunn rmunn self-assigned this Aug 14, 2024
Copy link

github-actions bot commented Aug 14, 2024

C# Unit Tests

66 tests   66 ✅  5s ⏱️
11 suites   0 💤
 1 files     0 ❌

Results for commit 31bed3b.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to expand the test cases for ConvertHgDate, otherwise it looks good.

backend/LexBoxApi/Services/HgService.cs Outdated Show resolved Hide resolved
backend/LexBoxApi/Services/HgService.cs Outdated Show resolved Hide resolved
Also expand the range of unit tests to include all kinds of invalid
values, making sure ConvertHgDate returns null for all invalid data.
@rmunn rmunn requested a review from hahn-kev August 15, 2024 02:26
hahn-kev
hahn-kev previously approved these changes Aug 16, 2024
We'll change this to upload build artifacts for the different container
logs, but this will get the error messages more quickly for now.
kubectl logs defaults to `--tail=10` if you pass a pod selector, so we
have to explicitly request `--tail=-1` to get *all* log lines.
@rmunn
Copy link
Contributor Author

rmunn commented Aug 19, 2024

Failing integration tests are caused by the hgweb container not being updated with PR code the way the lexbox-api container is. So when the API tries to call the new tipdate command, the hgweb command runner replies with HTTP 400 because it has an older version of command-runner.sh that doesn't include tipdate in the allowed_commands list.

@rmunn rmunn requested a review from hahn-kev August 19, 2024 11:15
@rmunn
Copy link
Contributor Author

rmunn commented Aug 20, 2024

Closing and reopening to get CodeQL to do its job, sigh...

@rmunn rmunn closed this Aug 20, 2024
@rmunn rmunn reopened this Aug 20, 2024
@rmunn rmunn enabled auto-merge (squash) August 20, 2024 04:40
@rmunn rmunn merged commit f23bc27 into develop Aug 20, 2024
18 checks passed
@rmunn rmunn deleted the bug/fix-last-commit-date-race branch August 20, 2024 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reproducible race condition in creating Mercurial project and uploading .zip file
2 participants