-
Notifications
You must be signed in to change notification settings - Fork 6
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
CKAN API used to validate the operationID #126
base: master
Are you sure you want to change the base?
Conversation
214ebfc
to
4b1d708
Compare
4b1d708
to
9025331
Compare
Code Climate has analyzed commit 9025331 and detected 0 issues on this pull request. View more on Code Climate. |
Hi Simran, Thank you for this pull request. Below are a few pointers for completing this PR. We enforce pep8 on this repo using the flake8 module. See here for an example of the errors - https://travis-ci.com/github/mapaction/mapactionpy_controller/jobs/498924357. Install and running Looking at the errors from builds earlier in the pull request, I think there are some problems with the unit tests: Ideally, the PR should include tests that demonstrate the correct behaviour of this new functionality. These should not make undue requests to a CKAN site just for the sake of testing. Because we need to maintain backward compatibility with Python 2.7, we can't use the most modern unit-testing and mock libraries. However, this is an example of a test using mocks from elsewhere in the code: mapactionpy_controller/mapactionpy_controller/tests/test_mapactionpy_controller.py Line 59 in f563dd1
Additionally, the new changes break some of the existing test (many of which use
I hope that is helpful. Thank you once again for your efforts. |
Aim:
This pull request solves the corresponding issue: CHEF_147 Jira Issue
I tried first getting familiar with the CKAN API, made the required changes for checking the validity of the operationID and then ran tests locally. If an operation ID is not valid, then an exception is raised.