Skip to content

Conversation

@miguelcleon
Copy link
Member

@lsetiawan I went ahead and made the fix for issue #132

see that the first one, this is caused by Dataloggerfiles model, mapping to the wrong column within the database: https://github.com/ODM2/ODM2PythonAPI/blob/master/odm2api/ODM2/models.py#L458.

For the second one, measurementequation is indeed misspelled: https://github.com/ODM2/ODM2PythonAPI/blob/master/odm2api/ODM2/models.py#L514.

@miguelcleon
Copy link
Member Author

why did this fail @lsetiawan @ocefpaf ?

@miguelcleon
Copy link
Member Author

also

found two more problems with dataloggerfiles model
There is no column dataloggeroutputfiledescription instead it is dataloggerfiledescription
https://github.com/ODM2/ODM2PythonAPI/blob/master/odm2api/ODM2/models.py#L460

There is no column dataloggeroutputfilelink instead it is dataloggerfilelink
https://github.com/ODM2/ODM2PythonAPI/blob/master/odm2api/ODM2/models.py#L461

@miguelcleon
Copy link
Member Author

I can confirm instances of these models can now be retrieved without error.

@lsetiawan
Copy link
Member

Sorry for replying to this so late. Was out sick yesterday. It seems like travis error is from Python 3.2 and 3.3 not being able to build the conda environment. Nothing to do with the changes. But, changes should go into the development branch rather than master. Please be advised. Thanks.

Copy link
Member

@lsetiawan lsetiawan left a comment

Choose a reason for hiding this comment

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

Looks good to me. Only thing, changes should be merged into development branch not master.

@lsetiawan lsetiawan force-pushed the master branch 2 times, most recently from 4acdf62 to 2f82702 Compare December 20, 2017 15:56

sudo: required

# if the https://travis-ci.org/ODM2/ODM2PythonAPI/requests ever says: missing config
Copy link
Member

Choose a reason for hiding this comment

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

@miguelcleon Any reason why these were added? It seems like your changes may have broken Travis CI.

Copy link
Member

Choose a reason for hiding this comment

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

Any changes to Travis should go through a PR so it doesn't just break without anyone knowing why. Thanks. I am going to roll back your changes to fix Travis.

Copy link
Member

Choose a reason for hiding this comment

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

Your changes are now in PR, please remove any change in .travis.yml

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this looks like a change I didn't mean to include.

@lsetiawan
Copy link
Member

Moved to #134. Closing.

@lsetiawan lsetiawan closed this Dec 20, 2017
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.

2 participants