Skip to content

Conversation

@michaelkaye
Copy link
Contributor

Rageshakes are too numerous for just uploading to github any more.

The generic webhook (and other) mechanism are able to track a greater volume of requests than simply uploading to github; however this means that we are going to be unable to use the issue ID generated by the github upload to track the rageshake.

We do create a unique ID for the on-disk rageshake, we expose this prefix as an ID in the objects we send onwards via the generic webhook API, which can then be used in external systems to uniquely identify this rageshake.

Technically this isn't any information that isn't in the ListingURL already, but i feel having a separate parameter storing the unique ID (such that it can evolve separately from the listingURL) is the right way to go.

This listing needs to be unique for the rageshake to be stored to disk,
so can be relied upon in future to be unique(enough) for other services
working with the rageshake.
@michaelkaye michaelkaye requested a review from richvdh as a code owner April 6, 2022 14:58
dfilename := strings.Trim(r.URL.Path,"/")
dfilename = strings.Replace(dfilename, "/","_",-1)
dfilename := strings.Trim(r.URL.Path, "/")
dfilename = strings.Replace(dfilename, "/", "_", -1)
Copy link
Member

Choose a reason for hiding this comment

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

for future reference: it would be great to put this sort of reformatting in a separate PR, so that we can easily see what's changing and what's not.

@michaelkaye michaelkaye force-pushed the michaelk/persist_unique_id_in_shakes branch from 9b0370b to 79454de Compare April 13, 2022 12:29
@michaelkaye michaelkaye requested a review from richvdh April 13, 2022 13:27
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm!

@michaelkaye michaelkaye merged commit 0223788 into master Apr 14, 2022
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.

3 participants