OKI code review

[pwalsh]

• tests are failing on travis
• Several issues with the client have created [epic] Improve split of the client from the cli code #43 for this
  • A seperation was attempted between CLI interface the application logic after my suggestion on the same. But, much of the client still uses click functions, and sys.exit, and in general is still written as if it is to be run in the shell. The result is that it is not very useful as a python client lib.
  • While do_publish and do_validate exhibit the above, do_configure is littered with print and does not, for example, do something useful like return the file path for the config, or the config object itself. Small things that could make a big difference if this is to be used as a python client
• DEFAULT_SERVER config with no way to override it?
  • Organize configuration options and defaults in the code better #44
• Can you pls use the OKI method for setting a package's version
• All the config fallbacks should be consolidated in the config file itself, and not spread in codebase. Example of config conditionals outside of config
  • Organize configuration options and defaults in the code better #44
• Publishing requires username and password on each publish action. Yet we do use jwt elsewhere. Please explain how auth/z works, and document it in the README. I don't understand why we'd want to use the password at all, once we have a jwt.
  • NOT SURE WHAT WE WILL DO - [doc] Add docs on authentication #45 is doc improvements only
• Please set up travis to deploy the package itself to pypi. See the OKI examples for how.
  • Automate deploy to pypi #71
• Related to my first point, all the tests depend on the CLI. Please test the CLI and the Python client.
  • [epic] Improve split of the client from the cli code #43
• As I've noted elsewhere, it is an unusual design to not properly validate data packages locally before pushing them to the server. We can completely avoid a whole class of user interactions, server error responses, and bad data quality, by doing this.
  • Data validation on the client #46
• The CLI only has two methods - validate and publish. The requirements for the CLI are quite clear and much more extensive.
  • We have no plans to implement all of this. We went with user stories that were more minimal and focused on publish only (adding delete and purge later).

[Fak3]

Travis build fixed (coveralls repo enabled after rename)

[rufuspollock]

@pwalsh

The CLI only has two methods - validate and publish. The requirements for the CLI are quite clear and much more extensive.

We have not had plans to implement all of this as we went with user stories that were more minimal and focused on publish only.

Publishing requires username and password on each publish action. Yet we do use jwt elsewhere. Please explain how auth/z works, and document it in the README. I don't understand why we'd want to use the password at all, once we have a jwt.

I'm still rather unclear on good practice with jwt. Giving out an infinitely lived token in the jwt seems to me problematic so we have a model where you have an access key that you can use (like an API key) and you submit that to get a jwt that you can use throughout a request cycle e.g. for publish.

As I've noted elsewhere, it is an unusual design to not properly validate data packages locally before pushing them to the server. We can completely avoid a whole class of user interactions, server error responses, and bad data quality, by doing this.

At the moment we do validate the json before pushing but not the data. Are you asking that we validate data too. Definitely something sensible to do but not MVP.

[pwalsh]

@rgrp the user stories are/where a way to order the requirements, not replace them. In any event, all but one of the API methods there are directly relevant to publishers: list, create, update, read, validate, delete.

[rufuspollock]

@pwalsh that was not how i had understood it. We did user stories and then picked prioritized ones. There is a bunch of stuff in the requirements that were not in the prioritized user stories. Let me be clear: I think requirements are good too - just not what we have been working off.

[pwalsh]

@rgrp currently, the validation of the DP is not profile-aware, and profile-specific validation is built into datapackage-py so is readily available. I think it would be useful. Likewise with data validation on the client (or server, if must) using goodtables or jsontableschema.Table - all the logic is there, and adding them to the client would ensure that the assumptions for tabular data on the server (vis, resource endpoints etc.) are safe assumptions to make.

[pwalsh]

@rgrp about long-lived JWT tokens, I'm not 100% sure myself. @akariv WDYT, and what do we do on OpenSpending?

[Fak3]

@pwalsh The dpm name is already taken on the pypi. Can we pick dpmpy or any other name to publish there? Or maybe someone can convince current owners to release it for us? (they seem to abandon it 4 years ago)

[pwalsh]

@Fak3 :). @rgrp owns dpm on pypi. It was the first ever implementation of this tooling.

[rufuspollock]

@pwalsh I'm still stuck re pypi because i don't have access to my account anymore. I've requested access to my account again here https://sourceforge.net/p/pypi/support-requests/685/ (but that was 2m ago and nothing has moved!)

[rufuspollock]

FIXED. I'm closed as fixed. Remaining items are either blocked e.g. #71 or are items that are planned but not immediate e.g. local data validation #46 or the work on other commands.