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

[mongodb] Simplify configuration, reduce to minimum #1771

Merged

Conversation

predominant
Copy link
Collaborator

@predominant predominant commented Aug 9, 2018

Signed-off-by: Graham Weldon graham@grahamweldon.com

tenor-113699260

This reduces the complexity of the configuration, and minimizes the default configuration. It also removes the configuration for mongos.

The new configuration is in line with the default configuration shipped with the MongoDB package for Debian systems.

This is in line with core-plans being a resource for bare minimum setup. More complex configuration should be done in configuration plans.

Testing

build; source results/last_build.env; hab pkg install results/${pkg_artifact}
hab svc load ${pkg_ident}

Service will load, then perform the following as a test of db interaction:

# mongo
MongoDB shell version v3.6.4
connecting to: mongodb://127.0.0.1:27017
MongoDB server version: 3.6.4
Welcome to the MongoDB shell.
For interactive help, type "help".
For more comprehensive documentation, see
	http://docs.mongodb.org/
Questions? Try the support group
	http://groups.google.com/group/mongodb-user


> show dbs;
admin  0.000GB
local  0.000GB


> use newdb
switched to db newdb


> db.createCollection( "contacts", {
...    validator: { $jsonSchema: {
...       bsonType: "object",
...       required: [ "phone" ],
...       properties: {
...          phone: {
...             bsonType: "string",
...             description: "must be a string and is required"
...          },
...          email: {
...             bsonType : "string",
...             pattern : "@mongodb\.com$",
...             description: "must be a string and match the regular expression pattern"
...          },
...          status: {
...             enum: [ "Unknown", "Incomplete" ],
...             description: "can only be one of the enum values"
...          }
...       }
...    } }
... } )
{ "ok" : 1 }

Signed-off-by: Graham Weldon <graham@grahamweldon.com>
@predominant predominant requested a review from a team as a code owner August 9, 2018 11:37
@thesentinels
Copy link
Contributor

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

@smacfarlane
Copy link
Contributor

Thanks for this PR @predominant ! While I personally think this is 100% the right pattern, I'm going to toss a needs discussion and do not merge label on this. We're removing a large swath of configuration that users of this plan may depend on that I think warrants some discussion before we merge this.

@predominant
Copy link
Collaborator Author

Thanks @smacfarlane, that’s what I anticipated :)

@nellshamrell
Copy link
Contributor

I also agree this is the correct course of action - thinking on how we can communicate this to users of the current plan who may depend on the configuration we are removing.

@nellshamrell
Copy link
Contributor

It also might be worth adding an example of how someone could use this in a configuration plan of their own.

Signed-off-by: Graham Weldon <graham@grahamweldon.com>
@smacfarlane
Copy link
Contributor

@nellshamrell Since the base plans refresh has been delayed, I'd like to target this to be merged as part of that work when we pick it up again. Thoughts?

@predominant
Copy link
Collaborator Author

Poke @nellshamrell and @smacfarlane on this one. Any update?

@smacfarlane smacfarlane added this to Open PRs in Core Plans Reviews Jan 7, 2019
@smacfarlane
Copy link
Contributor

@predominant I'm going to put this in the base plans refresh column of the project board and incorporate this work at that time.

@smacfarlane smacfarlane moved this from Open PRs to Base Plans Refresh - Hold in Core Plans Reviews Mar 13, 2019
@predominant
Copy link
Collaborator Author

@smacfarlane I'm cool with that. This has hung around for a while, I think we're all for including it, but were not sure when it should be done. I think at refresh time makes sense.

Thanks!!

tenor-186455917

@predominant
Copy link
Collaborator Author

@smacfarlane I propose we put this on the radar for base plans refresh as discussed. Would you like this done as a PR to the refresh branch, or shall we do it post-refresh?

@smacfarlane
Copy link
Contributor

@predominant GitHub lets repo owners change the target branch of a PR, so when we're ready we can merge this straight into the refresh branch. I've been holding on doing this until the glibc and gcc updates are ready, but it might be best to get this and the handful of other PRs in first.

@smacfarlane
Copy link
Contributor

@predominant I'm going to change the base branch on this and merge it into the refresh branch. The messaging you get when perform that operation warns about losing commits, but isn't clear on what might cause it. I've checked out a local copy in case something goes wrong, but don't expect it to since the target has the same HEAD as master and there are no conflicts.

@smacfarlane smacfarlane changed the base branch from master to refresh/2019q3 June 19, 2019 19:40
Core Plans Reviews automation moved this from Base Plans Refresh - Hold to Approved Jun 19, 2019
Copy link
Contributor

@smacfarlane smacfarlane left a comment

Choose a reason for hiding this comment

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

I believe this aligns with the intent of https://github.com/habitat-sh/core-plans/blob/master/docs/dev/policy_documents/deprecating-packages.md, formally RFC9 and am incorporating this work into the upcoming base plans refresh.

@smacfarlane smacfarlane merged commit f6f0928 into habitat-sh:refresh/2019q3 Jun 19, 2019
Core Plans Reviews automation moved this from Approved to Built Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants