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

Support for connection to remote mongodb #235

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

standa4
Copy link

@standa4 standa4 commented Jan 21, 2016

These changes require improved versions of hpfeeds and mnemosyne (pull-requests were created).

@r3k2
Copy link

r3k2 commented Apr 3, 2016

this seems like a good feature.. why is not going into the master branch?

@jatrost
Copy link
Collaborator

jatrost commented Apr 3, 2016

The main reason is it doesn't have authentication. If it had auth we would test it.

@r3k2
Copy link

r3k2 commented Apr 3, 2016

@jt6211 that makes sense. hopefully @standa4 will add auth to it.

@standa4
Copy link
Author

standa4 commented May 25, 2016

Sorry for delay,
I added support for authentication. Right now it works with mongodb version 2.6 (authentication method is MONGODB-CR). I have plans to modify it for newer version of mongodb but I think that there will be problem with hpfeeds feedbroker. It uses custom mongo client (evnet.mongodb) and it does not support newer auth methods. But right now it should work and you can test it.

@gregtampa
Copy link

I love this

@jatrost
Copy link
Collaborator

jatrost commented May 30, 2016

These changes look pretty good. Can you make the following change?

  1. In mhn/server/mhn/init.py, add:
def new_clio_conection():
    return Clio(
        mhn.config['MONGO_HOST'],
        mhn.config['MONGO_PORT'],
        mhn.config['MONGO_AUTH'],
        mhn.config['MONGO_USER'],
        mhn.config['MONGO_PASSWORD'],
        mhn.config['MONGO_AUTH_MECHANISM']
    )
  1. And then replace all those Clio(...) object creations with mhn.new_clio_conection(). This will make this much easier to maintain if we have to make changes with this again.
  2. Why make the MHN hpfeeds mongo config different for MHN vs mnemosyne? It seems like there should just be one mongo instance for these. When the install gets more complicated people make mistakes and their installs fail. Any ideas on how we could make this simpler?

@standa4
Copy link
Author

standa4 commented Jun 2, 2016

  1. I am working on these changes but I still need to test them. I will let you know when I will finish.
  2. Regarding second point, what about to create another install script that will be called first. In this script could be asked for all configuration questions and answers could be stored in some tmp file. Other install scripts will look for this file and if it exists, script will use values from it. Otherwise script will ask for configuration as usual. What do you think of this solution?

@jatrost
Copy link
Collaborator

jatrost commented Jun 3, 2016

This might be more complicated then you're willing to contribute, but ... My preference would be to only set the ENV variables in one place and have them take effect on all processes that used them anytime someone changed the variables in that one file and restarted the processes.

  1. create a file in /opt/mhn/ called environmentrc that would contain some ENV vars that represent the mongo options (and later we could add other MHN related ENVs as needed).
  2. modify anything that uses mongo to check the ENV variables named in this file (os.getenv('MONGO_HOST')).
  3. modify the supervisor scripts that manages these processes so it loads the /opt/mhn/enironmentrc file for any processes that use mongo to load the variables from this file.

@standa4
Copy link
Author

standa4 commented Jun 16, 2016

I made some tests and it looks like there is no problem and it works.
Regarding changes you suggesting, I can look at it later, but right now I don't have the time. Sorry. But I briefly checked if it would be feasible and I found one problem. Supervisor can't read environment variables from file. So one solution would be put ENV vars directly in supervisor script, but only on one place in supervisord section. Other scripts then inherit those variables (Supervisor/supervisor#771).

@d1str0
Copy link
Collaborator

d1str0 commented Mar 14, 2019

@standa4 do you know if this still works with the latest code? If so I will gladly merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants