update documentation for DAO config file setup. #199
Conversation
|
Why did this fail @lsetiawan ? It is just a simple documentation change. Maybe Appveyor just needs to be re-run? |
|
Yea, appveyor needs rerun. @ocefpaf help with this? Thanks! |
| @@ -235,7 +235,9 @@ cd $HOME/wofpyserverdev | |||
| 3. Edit `singlerunserver.py` to use the mysql config file. | |||
|
|
|||
| ```python | |||
| dao = Odm2Dao(get_connection('odm2_config_mysql.cfg')) | |||
| parser.add_argument('--config', | |||
There was a problem hiding this comment.
This is basically doing the same thing. Not sure what the difference is. See: https://github.com/ODM2/WOFpy/blob/master/wof/examples/flask/odm2/timeseries/runserver_odm2_timeseries.py#L42
There was a problem hiding this comment.
I think maybe a change to specify that one needs to put in the correct path to the .cfg would work better.
There was a problem hiding this comment.
@lsetiawan, FYI, I won't chime in on this unless you ask me to 😉 It would take me a good chunk of digging to be able to say anything intelligent about this. You're the guru here!
There was a problem hiding this comment.
@lsetiawan the difference is for me when I did this
dao = Odm2Dao(get_connection('odm2_config_postgresql.cfg'))
this line of code failed with an error but when I added it as the default in the parser argument it worked
There was a problem hiding this comment.
dao = Odm2Dao(get_connection(args.config))is basically using the value from args.config, which in your change, if the user doesn't specify --config will be 'odm2_config_mysql.cfg'. So this means that args.config is 'odm2_config_mysql.cfg'. So the runserver script ends up running, which is my original documentation:
dao = Odm2Dao(get_connection('odm2_config_mysql.cfg'))The breakage comes if 'odm2_config_mysql.cfg' is not in the same folder as the runserver script. So I think a change of maybe adding os.path.join(config_folder, 'odm2_config_mysql.cfg') is maybe better and explain that with that, one has to know where the config file lives. or if same folder, one can use os.curdir.
Makes sense? Let me know if that's confusing.
@emiliom do you think changing the default to 'odm2_config_mysql.cfg' is better than straight up changing the function that actually take the value? Thanks.
There was a problem hiding this comment.
because args.config is None on the line I listed above
There was a problem hiding this comment.
so the better change would be:
configfile = os.path.join(cfgpath, 'odm2_config_timeseries.cfg')
dao = Odm2Dao(get_connection(configfile))
app = wof.flask.create_wof_flask_app(dao, configfile)There was a problem hiding this comment.
@lsetiawan yes, that is it 😄
There was a problem hiding this comment.
@lsetiawan Seems kind of better to change it in one place rather then two?
There was a problem hiding this comment.
That's fine, but you won't be able to show that it should be from absolute path. Maybe I'll change that in the actual runserver script. Thanks!
|
The core problem has been figured out, merging now. |
|
AppVeyor now is working after the merge 👍 |
|
Isn't that strange? Why would an edit to one markdown file lead to a CI failure?? Oh well. I'm not worrying about this, until it happens again :| |
|
There was a choke when creating the conda environment. Nothing to worry about. Happens sometimes, just need to be restarted. Only @ocefpaf can do that. |
|
😌 |
Closing and opening should do the trick, but I am in the process of moving all the ODM2 AppVeyor instances to a "odm2" account that all the devs will have access. More on that later this week. |
|
@lsetiawan and @emiliom I am having trouble creating an ODM2 AppVeyor account. See appveyor/ci#1863 Others are experiencing the same issue and no there is official answer yet :-( |
|
Ok, thanks for the update. |
This fixes documentation problem referenced in #195 @emiliom