-
Notifications
You must be signed in to change notification settings - Fork 1
Dev Meeting Notes
- Meeting URL: http://bit.ly/ckandevmeeting
- Meeting pad: http://pad.okfn.org/p/ckan-meeting
Chair: Nigel
Notes: Adrià
- #1711 wardi: package memebership to orgs is defined both in member table and owner_org field, probably to ensure uniqueness. Also group_dict_save does a commit half way which looks really bad (https://github.com/ckan/ckan/blob/master/ckan/lib/dictization/model_save.py#L426) wardi to investigate
- #1668 nigel
- #1659 needs some tests
- #1665 Add datasets to any group regardless of membership. Do we need a config option or users should use custom auth functions (nigelb says that won't be enough
- #1715 Assigned to Ross
- #1713 Ask user to test on master and see if it's still fails
- #1712 Assigned to vitor
- #1710 Ask user for a patch
- #1709 How do we manage multiple extras in multiple resources? Assigned to adrià
- #1708 Ask user for a patch, assigned to joe
- #1706 leave as is
- #1705 remove /dataset/search endpoint. Ask more details about user extension that might be causing the issue
General Questions:
-
amercader: https://github.com/ckan/ckan/issues/1714 https://github.com/ckan/ckan/blob/master/ckan/logic/action/get.py#L2053 on CLI commands, session not handled as well as a web response. Try to change commit_and_remove on get_site_user by commit. Joe to look into cli db initialization field
-
wardi: organization_update packages again - adria says no more special behaviours for group/org_update for users field Investigate if _patch actions can be helpful here
- Chair
- amercader
- Notes
- nigelb
- #1665: nigel already reviewing.
- #1679: new type of action. should probably be in its own file. lets you provide a json patch to update a package. allows us to do atomic updates or updates that don't clobber each other. Ian wants feedback and someone else to take the issue :) json patch is list of operations. there's move, test, etc. So you can test your patch if a certain parameter fails. You can make it fail if it isn't what you wanted it to be. You can update a resource object or add a resource. If you want to update a whole resource you can do that. David asks if we would use revision id, Ian clarifies that is not how it's done. We're using a json patch library. It needs to be wrapped in things. The four lines it is right now is not complete. This is not atomic. If we want it to be better, we can build the patch into package_update. All of the business logic will need to be explicit, like about resources. The complexity will be in package_patch. We will need to consider what happens when someone sends an empty list of extras. We don't want to break anything. package_update should stay as is. It could potentially provide atomic updates to our user. There's more work, like it's own module and validation. Ian will stay assigned. Generally agreed it'll be really cool. The sooner we integrate, the better.
- [#1696](https://github.com/ckan/ckan/1696) Groups authorization bug. Nigel to continue working on.
- #1251 ongoing
- #1697 Ian to look again
- #1677 Vitor already reviewing
- #1512 Vitor will review. Need to check about localization.
- #1688 Adria to look
- #1617 assigned already
- #1605 assigned already
- #1659 need to talk about tests
- #1704 Need to take a look at library support. Allowing HTML is scary. Assigned to Nigel.
- #1701 If you're building a filter with dates, you'll get a lot of datasets with index date. We could also have 2 fields, one for string. And one for indexed. But the date field in solr will still be a proper date field. Adria already has a patch. We can discuss more.
- #1700 Vitor and Nigel to figure out. * ubuntu 14.04 support - to be decided about packages. We don't support ckan 1.8 unless there's a massive security bug.
General Questions:
- (amercader) Feedback on CKAN surveys. Add stuff, remove? http://pad.okfn.org/p/ckan-surveys
- (ian) functional tests with webtest: We need a way to test controllers. The suggestion is to start using webtest again, because in the scenario we don't care about js. Unless there's opposition, we'll use webtest with BeautifulSoup. Let's write one example with a pull request to discuss next week. Nigel to do this. David Read already has an example: https://github.com/ckan/ckan/blob/master/ckan/new_tests/controllers/test_util.py
- (nigel) Template tracebacks. Nigel wrote pull request to catch all exceptions so templates don't blow up. We should do a wrapper or something. Adria thinks helper should catch all expected exceptions. Catch the exception but re-raise it if debug mode is on. Otherwise make sure it sends an email. Ian wants the emails sent out. And about failures that may be related to us catching the exceptions. Agreed that catching all exceptions is a bad thing.
- (vitor) Logic about querying models outside of the models themselves: There were few places where we're querying models, like cli.py. Should we refactor that into models themselves. Adria agrees that this is a good idea. If the method from the model returned a query, we could add more filters. The might raise DetachedInstanceError. Vitor will bring up on ckan-dev about a more concrete example.
- (ian) vdm removal + sql for clearing old revisions (support from statcan). The options Ian had were clearing all revisions or dumping the packages into a clean database. Re-creating datasets might change UUIDs. David has a script that delete revisions (It's incredibly hard to do). Ian will try to convince new people to work on removing vdm.
- Clearing harvest sources (purging all its datasets) https://github.com/ckan/ckanext-harvest/blob/master/ckanext/harvest/logic/action/update.py#L110
- (ian) rename new_tests->tests and tests->legacy_tests: +1
- Chair
- joet
- Notes
- vitorbaptista
- #1665 - take care with how this changes the organization permissions as well (nigelb)
- #1679 - everyone agrees that it's a cool feature, but needs more discussion. we will revisit, please add your comments to the PR. (wardi)
- #1697 - needs more convincing. vitor will add a good test name that's over the line length limit to prove the point (vitorbaptista)
- #1659 - 2-stage dataset create is done, just missing (frontend) tests (vitorbaptista)
General Questions:
- nigelb: Do we land UI changes without tests or do we write old tests rather than no tests?
- we wondered what' s the issue with webtest. Nigel suggested doing integration tests with webtest through the controllers, and capybara (or something else) for pages that need javascript.
- Ian would like to unit test controllers. Nigel disagrees, saying that controllers are an integration point, so we shouldn't test them in isolation. Using webtest for controllers is fine. And we can use something for integration tests with javascript later.
- All in all, we felt webtest was good, but we would like to hear seanh/amercader opinions on this. Nigel'll mail ckan-dev talking about it.
- vitorbaptista: pull requests with commit messages not following our guidelines
- ask the person sending the PR to read http://docs.ckan.org/en/latest/contributing/commit-messages.html
- Chair
- amercader
- Notes
- nigelb
- #1692 assigned to vitor
- re: clean_action pull requst, it breaks calling dataset_show instead of package_show
- #1696 nigel to take a look. reproducable.
- #1694 sean will fix.
- #1598 If you hit the back button in the third page, the data in the third page is lost. The two state data creation might help. The state gets messed up by the package controller. There's no easy solution. We could use hidden fields without breaking customization.
General Questions:
- amercader: Proposal, dev polls. To know more about the ecosystem, help decide roadmap and recruit help
- Deployment: What OS do you use, Source install or package, How do you find the install instructions? Solr/ Postgres same machine, What OS would you like to see supported, Would you be willing to help test new releases on your distro? - generally agreed this is a good idea. amercader will setup a demo by next week.
- Chair
- wardi
- Notes
- nigelb
- #1692 sean assigned
- #1617 discussion continued, sean to merge soon
- #1685 joe
- #1684 joe
- #1683 wardi
- #1682 wardi
- #1677 vitor
- #1679 nigelb
- #1680 vitor
- #291 nigelb
- #1673 wardi
- #1503 sean to add new text
- #1671 nigelb
- #1693 sean to reply
- #1691 Documentation talks about old version of UI. Assigned to vitor.
- #1690 Activity stream can be disabled, but not revisions. Sean to close.
- #1688 wardi was trying get support for idea of removing draft stage. marked as good for contribution.
- #1687 context should not be in templates because of how it tightly coupled. Use get_action helper instead. Jinja has scoping problems when you use set. In other frameworks you would use a dictionary. c is messy and hard to track. Generally agreed that not using c is a good thing.
- #1686 But calling action functions from template not liked generally.
- #1681 It's kinda hard writing an expression to catch all domains and all email possibilities.
- #1678 wardi fixed it.
- #1676 assigned to adria
- #1675 general instructions/fixing https issues. talk to other people who have deployed ckan.
General Questions:
- [ckanext-realtime plugin by Justas Azna (reederz)] ( https://github.com/alexandrainst/ckanext-realtime ) Jutas joined us to talk about it. It observes datastores in real-time. It can makes websockets available to observe datastore. It's an internship project. There's documentation and tests. Once you setup the datastore, you need to make an API call which inserts a trigger. Uses postgres to listen to real time changes. There's a datastore listener script which listens for this, which sends to ckan to redis which sends to websocket server. There's a twisted eventloop between redis and websocket. Just calling the API makes it observable. It has some limitations. Currently, there can only be one websocket connection because it needs authentication and one API key can only get one connection. Justus is going to change it. It wouldn't be an additional load. May need to add multiple websocket servers.
- vitor: convert issues to pull requests - use "hub pull-request -i " (https://github.com/github/hub). Easy to convert issue to pull request. P
- Chair
- seanh
- Notes
- Ian
- #1667 break long title words - nigel to review
- #1666 fix for serious errors to data pusher - need a decision on front end tests to add a test, joe to review
- #1503 two-licenses issue - continued discussion on ambiguity, discusson to continue on pull request
- #1664 package_create/update auth fix - adria to review, sean to comment on how to add test
- #1251 resource views - continued work, adria to review
- #1663 remove repeated string - vitor assigned
- #1617 DetachedInstanceError - when new session is created old one is closed automatically, leading to a DetachedInstanceError later on, nigel has a fix but further explanation is requested. deep ckan knowledge required
- #1656 paster db clean fix - seanh assigned
- #1412 regenerate api keys - needs more tests joe reviewing, currently only usable from the front end. multiple api keys? possibly in the future
- #1668 deleted user bug - nigel assigned
- #1665 group auth for logged in user - possibly not complicated to implement, need to investigate why feature was removed in the first place - vitor volunteered
- #1662 can't deprecate with maintain.deprecated - likely decorator broken on object or controller method, possibly not designed for use around actions - joe to take a look
General Questions:
- joet: turn off openid by default? - to proceed
- joet: passlib is ok? https://github.com/ckan/ckan/pull/1658 agreed a great idea, work to continue and multiple reviewers to be requested when ready
- nigel: email hash https://github.com/ckan/ckan/pull/1657 helper function vs. special context vs. sysadmin access to user_list action, nigel to work on removing from api while still supporting gravatar in the user list controller
- seanh: DetachedInstanceError questions, discussion of controller, logic, model layers. discussed adding to description to contribting docs along with guidance about how to avoid replicating old code. session and model objects in context? are they singletons? No: session is per-request. model is passed because model.session might create a new session or return the existing one. model is also possibly good for mocking in tests.
- Chair
- Adria
- Notes
- Ian
- #1656 joe to add some notes to docstring
- #1412 joe to look at it
- #1617 ready for review, assigned to sean
- #1655 nigel assigned
- #1654 wardi assigned
- #1652 nigel assigned
- #1651 ubuntu 14.04 LTS support, ongoing
General Questions:
- nigelb: Documentation or UI for revisions. https://github.com/ckan/ideas-and-roadmap/issues/41 proposal to merge the revision information into the activities. First, revision/diff page can be resurrected https://github.com/ckan/ideas-and-roadmap/issues/33 https://github.com/ckan/ckan/pull/1590 based on activity and resource data that already exists in the database
- amercader: coveralls comments: can we turn them off? coveralls creates a lot of noise in emails, particlarly for small changes. can we set a threshold for the % change that creates an notification? yes https://coveralls.io/docs/notifications can we get notifications if it drops below 80%? vitor to investigate
- wardi: workflow plugins based on activities model: https://github.com/ckan/ideas-and-roadmap/issues/45 the future! we just need funding (wardi) JSON Patch is a thing and has a python implementation http://jsonpatch.com/ https://tools.ietf.org/html/rfc6902
- vitor: pull requests that lower our test coverage implement a policy on acceptance of patches that lower coverage? vitorbaptista is going to write some guidelines into the contributing docs and core devs will be stricter with their own PRs
- Chair
- Adria
- Notes
- Nigel
- #1251: sean is looking
- #1641: still WIP.
- #1550: nigel still working
- #1539: adria got volunteered at the last meeting
- #1649, #1648, #1647: All of them are correct. Someone needs to review and fix it. We'll ask Ross to take a look and review on Tuesday again if he doesn't want to.
- #1646: nigel
- #1643: It may potentially need to be removed. A type of homepage might show facets, need digging. This also assumes that there's only one type of package. It could be used in a different type of home page. We could change it so that you can request facets instead of having them declared in your controller.
General Questions:
-
CLA Feedback.
- Background: As we started the CKAN Association and moved to encouraging more contributions - we needed to get the legal status of CKAN core code on better footing. We (mostly Ira) explored both licence agreements and copyright assignments. Legally we are better protected with copyright assignment (presenting a united front to any who may want to challenge / sue), but data.gov.uk and others presented a strong preference for licencing agreement instead. We also reviewed all the various options that other open source projects and foundations use (usually a mix of the two). Ira talked lots to the JQuery Director who both have copyright assignement (for core devs) and licence agreements for others. After talking to the OKF lawyer, we decided to re-use their agreement (see here: http://contribute.jquery.org/CLA/) which they gave us permission for, with a few small tweaks our lawyer suggested. This meant that we take on less risk, using an agreement that already exists and has been used ‘in the wild’ and hence has set a precedent for how any legal challenges and complications are handled and how well this stands up in court etc.
- From Meeting: We all agree that we need but the proposed one has issues raised on the repo. The issues raised are not resolved yet. The issues would prefer to be closed when the person who opened it is happy with the response, but we shouldn't bikeshed and decide on a process. Rufus has responded and the dev team will chase it. Most people who have contributed have been employed by Open Knowledge, so historical contributors aren't that big of a problem. Going forward, we need to get it setup for new people contributing. How does this affect extensions? How are things on github.com/ckan considered core and do we make github.com/ckan similarly licensed and marked as core? Extensions need to be published anyway under AGPL. Someone that has been given the CKAN produce re-licensed would have difficulty with extensions. Anyone who created an issue on the cla needs to reply about issue, so we can keep the discussion moving. nigel to arrange meeting to discuss remaining issues in 2 weeks.
- Office hours. Rufus suggested and the people who want to come in are not peple who want to discuss things. Related issue people in the US who want to make it to the meetings, but the meetings are difficult for the US. From a user POV, this is great, but dev POV, this may end up being dedicated support time. The name is important and "office hours" might convey the wrong meaning. The discussions that we wanted part of it sounded good. Rufus is willing to step up and run a community hangout but needs someone from the tech team to help out. People should propose what they want to talk about. Ira suggested that this needs to be about QA, roadmap, etc. We generally think this is a good idea.
- (adam) Persistent Hangout? Adam to setup a persistent hangout. - https://plus.google.com/hangouts/_/okfn.org/ckan-dev-call
- (amercader) Supporting Ubuntu 14.04. Should we support 12.04 and 14.04 at the same time? It also switches to Python 3. Adria will create a new issue and we can start tracking this in there. Do we drop support for 12.04? We can't drop support easily. We could perhaps setup travis for 14.04 and also think about packages.
- (ira) Frequency of server or internal errors on demo; is this across all of ckan? who looks at logs? why is this so flaky? Demo is flaky for various reasons. Nigel to talk to Ira. David suggested setting up sentry.
- (amercader) 2.2.1 needs to happen. There are enough bugs to justify this. If you think something needs to be backported, tag it as such.
- (ira) possible for previews for what the resource file looks on the dataset page. Like a thumbnail of it. Potentially a great extension. If there's already a graph, a thumbnail of it will be really engaging. Potentially do it from a datapusher sort of thing in an extension. Ira to go and get lots of money to get it done!
- (wardi) moar cla discussion (only if there is time). Ian talked to devs at open contracting. They had seen CKAN but thought it was way too complicated and used Django instead. There wasn't a lot of compelling reasons. This adds danger to CKAN as a project. That'll give competitors the advantage. The copyright assignment bits are the dangerous bits. Was the lawyer's concern about copyright assignment? Copyright is not a very good open source citizen thing to do. The lawyer said this is the most legally defensible position. It is what others are using. It's not balanced, OKF has more rights than everyone else. Copyright assignment vs licencing. In a CLA, you give license, not give copyright. The difference is whether you actually assign copyright.
If there's a chance of someone going to pay to relicense CKAN, it's enough money to rewrite the whole thing. Ira is fairly happy as long as we have some clarity. Ira will look at DCO and see if it meets the things that Ira was worried about. Recommended reading: https://github.com/ckan/cla/issues/2. It's important to capture the questions and motivations of the CLA.
- Chair
- Nigel
- Notes
- Everyone
- #1641 resource_create should break when called without URL (vitorbapista)
- #1644 - add xlsx tsv to datapusher defaults (joet)
- #1617 - detached instance error (seanh)
- #1646 - nigelb will reply
- #1645 - should be part of a larger discussion about the api, is it consistent with package_show, user_list, etc. The count of the number of packages should be consistent as well, and not clash with another field in another api call - wardi
- #1643 - seanh
General Questions:
- (amercader) Meeting notes to wiki: Tricky, as no easy way to export to Markdown. Tried this but not great, as issues #xxx are translated as headings. Perhaps Etherpad allows export to Markdown? - (generally agreed)
https://docs.google.com/document/d/1MRNmhqBthZILBF8oG1NkwmyofRfOXr3NRJA1CCuHVD4/pub