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

Rate limiting (for a zone) #161

Merged
merged 3 commits into from
Feb 20, 2018
Merged

Conversation

benjvi
Copy link
Contributor

@benjvi benjvi commented Feb 13, 2018

Adds support for the rate limiting API. Mostly standard CRUD calls to API with marshalling etc. For listing, I allowed the caller to specify pagination options or call ListAll directly

Copy link
Contributor

@elithrar elithrar left a comment

Choose a reason for hiding this comment

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

LGTM.

rate_limiting.go Outdated

import (
"encoding/json"
"github.com/pkg/errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the style elsewhere in this project (presumably because most people are using goimports) is to put external imports at the end, after a blank line.


import (
"fmt"
cloudflare "github.com/cloudflare/cloudflare-go"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

rate_limiting.go Outdated
// RateLimit is a policy than can be applied to limit traffic within a customer domain
type RateLimit struct {
ID string `json:"id,omitempty"`
Disabled bool `json:"disabled,omitempty"` // api defaults to false
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if the comment is necessary here as bool also defaults to false

@benjvi
Copy link
Contributor Author

benjvi commented Feb 16, 2018

Thanks guys. I addressed the comments from @jamesog, also I realised today that the empty struct values were different than the api defaults when completely omitting a field in a couple of places -
which results in the wrong thing happening when empty values are given. I replaced these with pointers so the values get omitted completely and default values are used. Hopefully this should be ok now?

@elithrar elithrar merged commit 46dee6d into cloudflare:master Feb 20, 2018
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.

4 participants