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

update documentation for DAO config file setup. #199

Merged
merged 1 commit into from
Oct 20, 2017
Merged

update documentation for DAO config file setup. #199

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/WOFpyODM2LBR_installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

I think maybe a change to specify that one needs to put in the correct path to the .cfg would work better.

Copy link
Member

Choose a reason for hiding this comment

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

@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!

Copy link
Member Author

Choose a reason for hiding this comment

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

@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

Copy link
Member

Choose a reason for hiding this comment

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

@miguelcleon

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.

Copy link
Member

Choose a reason for hiding this comment

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

because args.config is None on the line I listed above

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

@lsetiawan yes, that is it 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

@lsetiawan Seems kind of better to change it in one place rather then two?

Copy link
Member

Choose a reason for hiding this comment

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

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!

help='Configuration file',
default='odm2_config_mysql.cfg')
```
3. Test that `singlerunserver.py` will deploy WOFpy. Go to one of the endpoints provided, replacing `127.0.0.1` with your server ipaddress:

Expand Down