Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

Add flag to override tokenLimit - #1265

Merged
jonboulle merged 1 commit into
coreos:masterfrom
miekg:limit-hack
Sep 23, 2015
Merged

Add flag to override tokenLimit#1265
jonboulle merged 1 commit into
coreos:masterfrom
miekg:limit-hack

Conversation

@miekg

@miekg miekg commented Jun 24, 2015

Copy link
Copy Markdown
Contributor

Make tokenLimit in api public and add a flag to fleet.conf (token_limit)
to override the built-in token limit of 100.

Note: this PR is rather hackish, it would be nice to know if something like will be considered for merging (if cleaned up, etc .etc.). IOW: comments welcome!

@bcwaldon

bcwaldon commented Jul 9, 2015

Copy link
Copy Markdown
Contributor

@miekg I'm on board with this change, but It would be cleaner to pass token limit through NewServeMux and hand it to the paginated resources directly rather than pulling it out of a package-level variable.

@miekg

miekg commented Jul 9, 2015

Copy link
Copy Markdown
Contributor Author

[ Quoting notifications@github.com in "Re: [fleet] Add flag to override to..." ]

@miekg I'm on board with this change, but It would be cleaner to pass token limit through NewServeMux and hand it to the paginated resources directly rather than pulling it out of a package-level variable.

Ack and ack. I hoping I have some time tomorrow to work on this.

/Miek

Miek Gieben

@miekg

miekg commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Friendly ping @bcwaldon

@mwitkow

mwitkow commented Sep 5, 2015

Copy link
Copy Markdown
Contributor

@mischief, can you please take at this PR reducing the size of tokenLimits. We use Fleet API from our services and the current 100 limit causes excessive load on etcd (each page is an etcd re-query).

@wuqixuan

wuqixuan commented Sep 7, 2015

Copy link
Copy Markdown
Contributor

LGTM

@miekg

miekg commented Sep 21, 2015

Copy link
Copy Markdown
Contributor Author

@wuqixuan can you merge? Thanks.

Comment thread fleetd/fleetd.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Maximum number of entries per page returned from API requests"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, the new statement is better.

@wuqixuan

Copy link
Copy Markdown
Contributor

@miekg , I have no permission to merge currently. Need @jonboulle @mischief help.

@miekg

miekg commented Sep 22, 2015

Copy link
Copy Markdown
Contributor Author

Ack, and I made that little change.

@jonboulle

Copy link
Copy Markdown
Contributor

@miekg could you please rebase + squash your commits into a single one (perhaps with the commit message from https://github.com/miekg/fleet/commit/3d3421d8b1f0080f41869988f18dc6a49b37a45a), then I will merge. thanks!

Add the tokenLimit to the NewServeMux and plumb it through the handlers.
Also add a flag to fleet.conf (token_limit) to override the built-in
token limit of 100.
@miekg

miekg commented Sep 23, 2015

Copy link
Copy Markdown
Contributor Author

done.

jonboulle added a commit that referenced this pull request Sep 23, 2015
Add flag to override tokenLimit
@jonboulle
jonboulle merged commit d7134cd into coreos:master Sep 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants