Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Add mongodb parameter module #1596

Merged
merged 4 commits into from
Apr 25, 2016
Merged

Add mongodb parameter module #1596

merged 4 commits into from
Apr 25, 2016

Conversation

nerzhul
Copy link
Contributor

@nerzhul nerzhul commented Feb 3, 2016

This module permit to configure mongodb live parameters to tune the running engine

@sivel
Copy link
Member

sivel commented Feb 3, 2016

This pull request has failed validation steps due to missing RETURN documentation. All new modules are required to add RETURN docs.

============================================================================
./database/misc/mongodb_parameter.py
============================================================================
ERROR: No RETURN provided

@nerzhul
Copy link
Contributor Author

nerzhul commented Feb 3, 2016

Fixed

@gregdek
Copy link
Contributor

gregdek commented Feb 5, 2016

Thanks @nerzhul for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion.

@nerzhul
Copy link
Contributor Author

nerzhul commented Feb 16, 2016

@gregdek nobody wants to review it ? :)


# import module snippets
from ansible.module_utils.basic import *

Copy link
Contributor

Choose a reason for hiding this comment

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

imho, you should add the script condition see

if __name__ ==  '__main__':
    main()

@Lujeni
Copy link
Contributor

Lujeni commented Feb 20, 2016

@nerzhul imo, missing params to support the mongodb authentication (authenticate, mechanism), could be usefull to support all kind of mongodb platforms.

@gregdek imo, we need a unique way to manage the mongodb connection, each module try to re-implement it

This module permit to configure mongodb live parameters to tune the running engine
@nerzhul
Copy link
Contributor Author

nerzhul commented Feb 21, 2016

@Lujeni i fixed the main call condition. Authentication is already okay, i import it from mongodb_user module. What is the problem then ?

@Lujeni
Copy link
Contributor

Lujeni commented Feb 21, 2016

The mongodb_user module doesn't support all necessary options like mechanism or ssl cert.

If we changed something in the mongodb_user module connection part, we need to re-check each mongodb modules, it's not dry. As i say in the previous comment, imo, we need a single way to manage the mongodb connection accross all mongodb module.

However, we can shipit this module 👍

@nerzhul
Copy link
Contributor Author

nerzhul commented Feb 21, 2016

Thanks @Lujeni :)
You are right. There is same problem on MySQL module in fact. If you have time to review #1593, don't hesitate

@nerzhul
Copy link
Contributor Author

nerzhul commented Apr 20, 2016

@Lujeni @gregdek can you update this PR status ?

@gregdek
Copy link
Contributor

gregdek commented Apr 20, 2016

Will put into shipit for further review. Thanks @nerzhul!

@nerzhul
Copy link
Contributor Author

nerzhul commented Apr 20, 2016

Thanks @gregdek !

'''

RETURN = '''
changed:
Copy link
Contributor

Choose a reason for hiding this comment

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

removed changed here, it is a common return. Only document the returns the module provides please.

module.fail_json(msg='Cannot run without value option')

if param_type not in valid_param_types:
module.fail_json(msg='param_type should have one of the following value: %s' % str(valid_param_types))
Copy link
Contributor

Choose a reason for hiding this comment

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

This check can be removed after choices=['str', 'int']has been added, see comment above

@resmo
Copy link
Contributor

resmo commented Apr 25, 2016

I would like to merge it into 2.1 (freeze is today!) please update asap as commented :)

@nerzhul
Copy link
Contributor Author

nerzhul commented Apr 25, 2016

@resmo updated !
Tell me if it's good for you, then i will squash

login_port=dict(default=27017, type='int'),
login_database=dict(default=None),
replica_set=dict(default=None),
param=dict(default=None),
Copy link
Contributor

Choose a reason for hiding this comment

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

add required=True to param and value

@resmo
Copy link
Contributor

resmo commented Apr 25, 2016

that was fast! no need to squash, github is able to squash commits natively :) the required=True are still missing IMHO

@nerzhul
Copy link
Contributor Author

nerzhul commented Apr 25, 2016

@resmo done :)

@resmo
Copy link
Contributor

resmo commented Apr 25, 2016

Thanks!

@resmo resmo merged commit d3a34c0 into ansible:devel Apr 25, 2016
@nerzhul
Copy link
Contributor Author

nerzhul commented Apr 25, 2016

Thanks !

jimi-c pushed a commit to ansible/ansible that referenced this pull request Apr 25, 2016
NicCOConnor pushed a commit to NicCOConnor/ansible-modules-extras that referenced this pull request May 1, 2016
* Add mongodb parameter module

This module permit to configure mongodb live parameters to tune the running engine
cinek810 pushed a commit to cinek810/ansible-modules-extras that referenced this pull request Jun 12, 2016
* Add mongodb parameter module

This module permit to configure mongodb live parameters to tune the running engine
Constantin07 pushed a commit to Constantin07/ansible-modules-extras that referenced this pull request Jun 28, 2016
* Add mongodb parameter module

This module permit to configure mongodb live parameters to tune the running engine
perzizzle pushed a commit to perzizzle/ansible-modules-extras that referenced this pull request Jul 12, 2016
* Add mongodb parameter module

This module permit to configure mongodb live parameters to tune the running engine
perzizzle pushed a commit to perzizzle/ansible-modules-extras that referenced this pull request Jul 12, 2016
* Add mongodb parameter module

This module permit to configure mongodb live parameters to tune the running engine
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants