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

Set up new gh ratelimit publisher for gateway/temporalworker #712

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

samrabelachew
Copy link
Contributor

@samrabelachew samrabelachew commented Jul 14, 2023

Two changes here:

  1. Inject installation ID defining a separate GH app for gateway and temporal servers (we will eventually reconfigure code to use these installation ids over the ones passed in from the webhook event)
  2. Set up new github ratelimit stats publisher in gateway and temporal servers that use these app installations. This already needed to be migrated over to the new workers anyways, but also can serve as an initial test of the new ids without having to impact existing code.

@samrabelachew samrabelachew changed the title Set up new gh ratelimit publisher for gateway/temporalworker and inject installation id as global configs Set up new gh ratelimit publisher for gateway/temporalworker Jul 14, 2023
"github.com/runatlantis/atlantis/server/neptune/sync"
"github.com/runatlantis/atlantis/server/vcs"
"github.com/stretchr/testify/assert"
)

const testRoot = "testroot"

type testAllocator struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleting this is an unrelated lint fix (not used code)

Comment on lines +32 to +35
installationClient, err := r.ClientCreator.NewInstallationClient(r.InstallationID)
if err != nil {
return errors.Wrap(err, "creating installation client")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this creating a new client every time? or does it pass back one thats already been created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ClientCreator comment:

// NewCachingClientCreator returns a ClientCreator that creates a GitHub client for installations of the app specified
// by the provided arguments. It uses an LRU cache of the provided capacity to store clients created for installations
// and returns cached clients when a cache hit exists.

See code itself for cache layer: https://github.com/palantir/go-githubapp/blob/develop/githubapp/caching_client_creator.go#L74

@samrabelachew samrabelachew merged commit 85f9720 into main Jul 14, 2023
2 checks passed
@samrabelachew samrabelachew deleted the installation-app-id branch July 14, 2023 19:45
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