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

Nevermind about the webhook secret alt #551

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Nevermind about the webhook secret alt #551

merged 1 commit into from
Jul 26, 2023

Conversation

chadwhitacre
Copy link
Member

@chadwhitacre chadwhitacre commented Jul 26, 2023

Part of #482, reverts #550.

I was thinking about it wrong, the "or" is at startup config time, not at runtime. We still only get one secret at runtime (I did a little poking and didn't find differently). Gonna just have to run the race condition I think. :-/

Rollout plan

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (564f7c3) 85.10% compared to head (e3f32c2) 85.10%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #551   +/-   ##
=======================================
  Coverage   85.10%   85.10%           
=======================================
  Files          97       97           
  Lines        2450     2450           
  Branches      478      478           
=======================================
  Hits         2085     2085           
  Misses        358      358           
  Partials        7        7           
Files Changed Coverage Δ
src/api/github/index.ts 75.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chadwhitacre chadwhitacre mentioned this pull request Jul 26, 2023
43 tasks
@chadwhitacre chadwhitacre merged commit c614c4f into main Jul 26, 2023
@chadwhitacre chadwhitacre deleted the cwlw/nevermind branch July 26, 2023 01:03
@chadwhitacre
Copy link
Member Author

Done! Tested in getsentry/self-hosted#2301, appears to be working. 👍

@chadwhitacre
Copy link
Member Author

Looks like we did get some dropped events, seeing 29.

Screenshot 2023-07-25 at 9 13 21 PM

https://sentry.sentry.io/issues/4340314075/

@chadwhitacre
Copy link
Member Author

chadwhitacre commented Jul 26, 2023

Here's a quick dump of red events that line up time-wise with the Sentry events:

  d3477e00-2b50-11ee-9808-aeb302250dab check_run.created…2023-07-25 21:07:44
  d34003f0-2b50-11ee-99dc-a29555da94d5 check_run.completed…2023-07-25 21:07:44
  d342ea20-2b50-11ee-915d-c89b852e120b check_run.completed…2023-07-25 21:07:44
  d3466c90-2b50-11ee-8768-aa98d3a66908 check_run.created…2023-07-25 21:07:44
  d2f5b480-2b50-11ee-9e1e-d9b667b62f8a check_run.created…2023-07-25 21:07:44
  d2ebf080-2b50-11ee-80be-8da2b33d7740 check_run.completed…2023-07-25 21:07:44
  d2ee3a70-2b50-11ee-9444-047373c8af0d check_run.completed…2023-07-25 21:07:44
  d2f00f30-2b50-11ee-87e3-7b7cecad5d1e check_run.created…2023-07-25 21:07:44
  d284ca40-2b50-11ee-9262-a2c7b90da82f check_run.created…2023-07-25 21:07:43
  d27fc130-2b50-11ee-952a-4febca4d0571 check_run.completed…2023-07-25 21:07:43
  d2736520-2b50-11ee-85cb-23ffc85a94a6 check_run.created…2023-07-25 21:07:43
  d26e0df0-2b50-11ee-9d08-fbf2ee535e8b check_run.completed…2023-07-25 21:07:43
  d1a76290-2b50-11ee-9af9-dde14195b651 check_run.created…2023-07-25 21:07:42
  d1a28090-2b50-11ee-8dd3-5679c130178b check_run.completed…2023-07-25 21:07:41
  d1a5b4e0-2b50-11ee-8a27-c98296fedeb8 check_run.created…2023-07-25 21:07:41
  d1559910-2b50-11ee-87c3-a8093dc35e84 check_run.created…2023-07-25 21:07:41
  d161f520-2b50-11ee-9a7f-12aee202b162 check_run.created…2023-07-25 21:07:41
  d163c9e0-2b50-11ee-9a48-36e8c8cdf652 check_suite.completed…2023-07-25 21:07:41
  d15cc500-2b50-11ee-89d5-1c6a4c6ced11 check_run.completed…2023-07-25 21:07:41
  d142d460-2b50-11ee-980f-d3b76b442418 check_run.created…2023-07-25 21:07:41
  d10703e0-2b50-11ee-860b-95db519e1816 check_run.created…2023-07-25 21:07:41
  d101fad0-2b50-11ee-9b5e-9edc05399cdb check_run.completed…2023-07-25 21:07:40
  d0ea2d10-2b50-11ee-9a87-48805945a993 check_run.created…2023-07-25 21:07:40
  d0ad7230-2b50-11ee-9505-29c5d5a3296b check_run.created…2023-07-25 21:07:40
  d0a275b0-2b50-11ee-8819-8303165f635f check_run.created…2023-07-25 21:07:40
  cfaf8a30-2b50-11ee-817b-f33e8949ac89 check_run.created…2023-07-25 21:07:38
  cf75b580-2b50-11ee-96b2-4528dc654b3b check_run.completed…2023-07-25 21:07:38
  cf550e20-2b50-11ee-9cfc-360cce643bd8 check_run.created…2023-07-25 21:07:38
  cf7196d0-2b50-11ee-9a50-8a46140545cd check_run.created…2023-07-25 21:07:38

I count 29. 👍

@chadwhitacre
Copy link
Member Author

Supposedly GETable.

@chadwhitacre
Copy link
Member Author

I expanded them all in the UI and copy/pasted them to a file.

$ grep 'Response 400' dropped-events.txt|wc -l
      29
$

@chadwhitacre chadwhitacre mentioned this pull request Jul 26, 2023
6 tasks
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.

2 participants