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

Add Mongo scaler documentation #343

Merged
merged 1 commit into from
Jan 26, 2021
Merged

Add Mongo scaler documentation #343

merged 1 commit into from
Jan 26, 2021

Conversation

NUCsimple
Copy link
Contributor

This PR adds documentation for Mongo scaler.
Relates to kedacore/keda#1465.

@NUCsimple NUCsimple mentioned this pull request Dec 31, 2020
Merged
4 tasks
@tomkerkhove
Copy link
Member

@NUCsimple Can you fix DCO please?

tomkerkhove
tomkerkhove reviewed Jan 1, 2021
View reviewed changes
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

It's a good start but needs a bit of alignment with our other scalers, thanks!

Also, can you change all mongo references to mongoDb please?

content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongo.md Outdated Show resolved Hide resolved
@NUCsimple NUCsimple force-pushed the master branch 2 times, most recently from bc28372 to 63ef293 Compare January 9, 2021 13:30
tomkerkhove
tomkerkhove suggested changes Jan 11, 2021
View reviewed changes
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

LGTM, just need a small change with respect to the product name - Sorry!

content/docs/2.1/scalers/mongoDB.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongoDB.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongoDB.md Outdated Show resolved Hide resolved
content/docs/2.1/scalers/mongoDB.md Outdated Show resolved Hide resolved
tomkerkhove
tomkerkhove approved these changes Jan 14, 2021
View reviewed changes
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Just waiting for the feature to be merged now.

content/docs/2.1/scalers/mongoDB.md Outdated Show resolved Hide resolved
zroubalik
zroubalik reviewed Jan 14, 2021
View reviewed changes
content/docs/2.1/scalers/mongoDB.md Outdated
maintainer = "Community"
description = "Scale applications based on MongoDB."
availability = "v2.1+"
go_file = "MongoDB_scaler"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
go_file = "MongoDB_scaler"
go_file = "mongodb_scaler"

this should match the actual name of the go file with the code

content/docs/2.1/scalers/mongoDB.md Outdated
# Required: query expr, used by filter data
query: '{"region":"eu-1","state":"running","plan":"planA"}'
# Required: according to the number of query result, to scale job
queryValue: "1"
Copy link
Member

@zroubalik zroubalik Jan 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Information about metricName is missing, please take a look on example info that's provided by other scalers:
https://keda.sh/docs/2.1/scalers/influxdb/ and https://keda.sh/docs/2.1/scalers/postgresql/

content/docs/2.1/scalers/mongoDB.md Outdated

```yaml
triggers:
- type: mongoDB
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- type: mongoDB
- type: mongodb

content/docs/2.1/scalers/mongoDB.md Outdated
successfulJobsHistoryLimit: 0 # Optional. Default: 100. How many completed jobs should be kept.
failedJobsHistoryLimit: 10 # Optional. Default: 100. How many failed jobs should be kept.
triggers:
- type: mongoDB
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- type: mongoDB
- type: mongodb

zroubalik
zroubalik reviewed Jan 14, 2021
View reviewed changes
content/docs/2.1/scalers/mongoDB.md Outdated

### Trigger Specification

This specification describes the `mongoDB` trigger that scales based on result of MongoDB query.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This specification describes the `mongoDB` trigger that scales based on result of MongoDB query.
This specification describes the `mongodb` trigger that scales based on result of MongoDB query.

zroubalik
zroubalik requested changes Jan 15, 2021
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

There are minor details to be fixed

@tomkerkhove
Copy link
Member

Any update on this @NUCsimple?

@zroubalik
Copy link
Member

@NUCsimple could you please fix this, so we can proceed with a release? Thanks!

@NUCsimple
Copy link
Contributor Author

@zroubalik I'll fix it today,sorry.

zroubalik
zroubalik reviewed Jan 25, 2021
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Looking good, there's minor nit in the metricName documentation

@NUCsimple could you please add this?

content/docs/1.5/scalers/mongodb.md Outdated
- `collection` - Name of the collection
- `query` - A MongoDB query that should return single numeric value
- `queryValue` - A threshold that will define when scaling should occur
- `metricName` - An optional name to assign to the metric.
Copy link
Member

Choose a reason for hiding this comment

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

Information about metricName is missing, please take a look on example info that's provided by other scalers:
https://keda.sh/docs/2.1/scalers/influxdb/ and https://keda.sh/docs/2.1/scalers/postgresql/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

@zroubalik
Copy link
Member

@NUCsimple I've just noticed that you placed the scaler docs in content/docs/1.5/scalers/mongodb.md could you please move it to content/docs/2.1/scalers/mongodb.md, as this scaler will be present from KEDA 2.1 onwards

@NUCsimple
Copy link
Contributor Author

@zroubalik ,I'm so sorry. Maybe the mouse moved the file by mistake just now

add mongodb scaler doc
cdceb3b 
Signed-off-by: 高强 <wb-gq555900@alibaba-inc.com>
@zroubalik zroubalik merged commit 0b1a7ba into kedacore:master Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zroubalik zroubalik zroubalik approved these changes

@tomkerkhove tomkerkhove tomkerkhove approved these changes

@ahmelsayed ahmelsayed Awaiting requested review from ahmelsayed

@jeffhollan jeffhollan Awaiting requested review from jeffhollan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NUCsimple @tomkerkhove @zroubalik