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.

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

Upgrade datapusher in inventory app #1422

Closed
10 tasks done
avdata99 opened this issue Feb 29, 2020 · 13 comments
Closed
10 tasks done

Upgrade datapusher in inventory app #1422

avdata99 opened this issue Feb 29, 2020 · 13 comments
Assignees

Comments

@avdata99
Copy link
Contributor

avdata99 commented Feb 29, 2020

Description

In the inventory-app we are using a fork of the official datapusher extension.

As a part of a plan to upgrade CKAN extensions in use, and in order to improve the way that we manage forks we need to upgrade this extension.

Acceptance Criteria

  • When there's a new release upstream, we can get onto it in under an hour
  • When there's a new release upstream, we see a notification in a Slack channel
  • If the upstream maintainers don't already use versioning, there's an issue in their tracker requesting they do.
  • If the upstream maintainers don't already have tests, there's an issue in their tracker requesting they add them.
  • We have a test environment for this extension

Tasks

  • Analyze which of the tagged versions are the last for CKAN 2.8. Avoid using CKAN 2.9 changes if they exist.
  • Create a test environment with the inventory-app
  • Test this extension
  • Measure the effort to move the inventory-app to this official version.
  • Connect our Slack channel with new release news in upstream

Analysis & notes

The official repo has tests and CI. It is a very active, well documented and widely used version in the CKAN community.

Our fork is Ahead in 11 commits and Behind 85 commits. This potential PR show us the changes of our 11 commits.

Lasts commit from Dec 2019 seems a kind of custom hotfix that maybe should be fixed in a different way and sent changes to upstream.

It's a good idea to define an environment with the inventory-app and just change the datapusher to the official version and run tests.

Recommendation

We recommend moving to upstream.
Analyze to move to xloader (a new extension designed as a replacement for DataPusher)

@adborden
Copy link
Contributor

One of the patches on datapusher is a switch from using application/json to application/x-www-form-urlencoded. I think this is because inventory ckan is using the wrong version of repoze.who==1.0.18. I think this means we'll want to address Inventory ckan before taking on datapusher (and then we can drop those patches).

@avdata99
Copy link
Contributor Author

One of the patches on datapusher is a switch from using application/json to application/x-www-form-urlencoded. I think this is because inventory ckan is using the wrong version of repoze.who==1.0.18. I think this means we'll want to address Inventory ckan before taking on datapusher (and then we can drop those patches).

The inventory-app, at the new branch inventory_ckan_2.8, we already are using repoze.who==2.3.

@avdata99
Copy link
Contributor Author

Work in progress PR. Not ready yet.

@avdata99 avdata99 self-assigned this Mar 12, 2020
@avdata99
Copy link
Contributor Author

We have 3 option here:

  • Move to XLoader: This new PR uses XLoader which is better for this.
  • Include Datapusher with code: This PR to use datapusher from code.
  • Include a buit image of datapusher: PR (ready to go)

The 3 options are working. The XLoader seems to be the best option here

@kimwdavidson
Copy link

Andres to connect 3rd PR and close other two

@adborden
Copy link
Contributor

I captured the recommendation to use ckanext-xloader in #1486, feel free to add additional details.

@avdata99
Copy link
Contributor Author

Closed PR for XLoader and datapusher with code.
Added a final PR
If approved, we can continue with the missing tasks

@kimwdavidson kimwdavidson added this to the Data.gov Sprint 18 milestone Mar 23, 2020
@avdata99
Copy link
Contributor Author

They usually change version numbers
image

@estebanruseler
Copy link

Synced with Andres so we move this from 90% done to 100% done. Agreed:

  • Double-check that we have requested semantic versioning and testing.Task-list to be updated.
  • The other o/s items are the Slack notifications. We will create a ticket for this to be considered as part of next sprint

@avdata99
Copy link
Contributor Author

avdata99 commented Mar 25, 2020

We have tests at upstream and they work fine.

image

We also add a test in docker

@estebanruseler
Copy link

Blocked by slack notifications. This should be easy and we just need to sync with Bret/Aaron on the scrum call.

@avdata99
Copy link
Contributor Author

avdata99 commented Mar 30, 2020

Added to releases RSS to datagov-notifications slack channel

/feed suscribe https://github.com/ckan/datapusher/releases.atom

@avdata99
Copy link
Contributor Author

avdata99 commented Apr 2, 2020

Closing since is done

@avdata99 avdata99 closed this as completed Apr 2, 2020
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

No branches or pull requests

4 participants