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.

Sign up for GitHub

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

Jump to bottom

Feature/rate limiting2 #1667

Merged
merged 10 commits into from
Oct 4, 2016
Merged

Feature/rate limiting2 #1667

merged 10 commits into from
Oct 4, 2016

Conversation

jykae
Copy link
Contributor

@jykae jykae commented Sep 30, 2016

Closes #1190

@jykae jykae added this to the Sprint 32 milestone Sep 30, 2016
brylie
brylie reviewed Sep 30, 2016
View reviewed changes
proxy_backends/collection/apiUmbrellaSchema.js
],
autoform: {
options: {
custom: 'Custom rate limits',
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to add i18n to these texts.

brylie
brylie reviewed Sep 30, 2016
View reviewed changes
proxy_backends/collection/apiUmbrellaSchema.js
'rate_limits.$.duration': {
type: Number,
optional: true,
label: 'Duration (ms)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember i18n here.

brylie
brylie reviewed Sep 30, 2016
View reviewed changes
proxy_backends/collection/apiUmbrellaSchema.js
],
autoform: {
options: {
apiKey: 'API Key',
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember i18n here.

brylie
brylie reviewed Sep 30, 2016
View reviewed changes
proxy_backends/collection/apiUmbrellaSchema.js
'rate_limits.$.limit': {
type: Number,
optional: true,
label: 'Number of requests',
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember i18n here.

brylie
brylie reviewed Sep 30, 2016
View reviewed changes
proxy_backends/collection/apiUmbrellaSchema.js
'rate_limits.$.response_headers': {
type: Boolean,
optional: true,
label: 'Show rate limit in response headers',
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember i18n here.

@jykae
Copy link
Contributor Author

jykae commented Oct 3, 2016

Reviewer, added RateLimit schema, cause sub-field translations did not otherwise work.

@brylie brylie self-assigned this Oct 3, 2016
@brylie
Copy link
Contributor

brylie commented Oct 3, 2016

This works, but does not alert the end-user of success. There was an sAlert onSuccess for the autoform, at one point.

@brylie
Copy link
Contributor

brylie commented Oct 3, 2016

This is also allowing the same data to be submitted multiple times to API Umbrella, causing duplicate entries in the API Umbrella Backends collection. It is probably related to the form not entering a 'success' state, which would also prevent the onSuccess callback from firing.

@jykae
Copy link
Contributor Author

jykae commented Oct 3, 2016

@brylie Could you help to reproduce issues mentioned above?

@jykae
Copy link
Contributor Author

jykae commented Oct 3, 2016

@brylie Discussion summary attached

Required changes

  • set default rate limit mode to "unlimited", both insert/update.

@jykae
Copy link
Contributor Author

jykae commented Oct 3, 2016 •
edited

@brylie Opened bug/enhancement with response_headers value, should be able to select like radio button, #1684

@brylie
Copy link
Contributor

brylie commented Oct 4, 2016

I will merge this.

@jykae please also open an enhancement describing your idea for improving the duration field to allow user to use 'minutes', 'hours', etc.

@brylie brylie merged commit 1fa2668 into develop Oct 4, 2016
@brylie brylie deleted the feature/rate-limiting2 branch October 4, 2016 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brylie brylie brylie left review comments

Assignees

@brylie brylie

Labels
None yet
Projects
None yet
Milestone
Sprint 32
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jykae @brylie