Update tag-git comment in cascading runs; Follow up
#49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…bust In my tests, it seems that the trailing newline may be shown when searching without any token, and trimmed when searching with token. That makes no sense, but let's guard against this condition anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In my tests, sometimes even though the 10 minutes maximum expire time
worked when searching for the comment, it still was possible for the
subsequent request (appending to the comment) to fail with:
'Expiration time' claim ('exp') is too far in the future
Let's work around that by avoiding the full 10 minutes expiry and going
for 9 instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
rimrul
approved these changes
Nov 15, 2023
| // JWT expiration time (10 minute maximum) | ||
| exp: now + (10 * 60), | ||
| // JWT expiration time (10 minute maximum, use 9 to allow for clock drift) | ||
| exp: now + (9 * 60), |
There was a problem hiding this comment.
I think the 10 minute maximum is supposed to be 10 minutes from iat, so with that being offset 60 seconds into the past, exp being 9 minutes in the future would make sense.
There was a problem hiding this comment.
I had the same thought, but why did it seem to work so far, then? It's a puzzle.
|
It worked!!! No manual interaction necessary, apparently this PR made the logic robust enough. Yay! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #47 I tried to teach GitForWindowsHelper to find the PR comment containing
/git-artifactswhen thetag-gitcheck run finished that was triggered by that slash command. However, it did not work, despite the webhook being delivered with status 200 and the response body being:I do not really know why it failed, but when I tried to run the code locally (commenting out the part that would have triggered another pair of
git-artifactsruns), it didn't work for me either, due to expire time issues ("too far in the future") and due to too-strict matching (sometimes the fragment in the search result ended in a newline character, sometimes it did not).This PR is designed to help with both issues, so that hopefully the v2.43.0 (final) PR will see the
/git-artifactscomment being updated as intended.At least when I ran the code locally with these modifications, the
/git-artifactscomment was updated as I wanted, if somewhat belatedly.