Skip to content

Use dpp-runner instead of babbage-fiscal#18

Merged
brew merged 18 commits intomasterfrom
feature/dpp-runner
Apr 16, 2018
Merged

Use dpp-runner instead of babbage-fiscal#18
brew merged 18 commits intomasterfrom
feature/dpp-runner

Conversation

@akariv
Copy link
Copy Markdown
Member

@akariv akariv commented Mar 12, 2018

This is a bit thin, as most of the modifications were done in babbage.

The main change there is to support a slight change to the babbage model syntax for supporting joins between two DB tables.

It used to support a simple "join-column": "bla" syntax, allowing to join two db tables with the same name.
The change in openspending/babbage@53e20b6 extended that to a 2-string array, detailing the names of the two columns in the two tables.

@akariv akariv requested a review from brew March 12, 2018 10:43
Copy link
Copy Markdown
Contributor

@brew brew left a comment

Choose a reason for hiding this comment

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

The babbage change still supports the single shared name syntax, as well as the new 2-string array, right? Looks good to me.

@akariv akariv force-pushed the feature/dpp-runner branch 3 times, most recently from 08677d5 to c680a32 Compare March 20, 2018 13:30
@akariv akariv force-pushed the feature/dpp-runner branch from c680a32 to 1a3f2cf Compare March 20, 2018 13:31
@akariv
Copy link
Copy Markdown
Member Author

akariv commented Mar 20, 2018

@brew I added what we discussed (removal of os-api-loader).
Please take a look!

@brew
Copy link
Copy Markdown
Contributor

brew commented Mar 21, 2018

Cool, a couple of issues with old babbage fiscal imports.

I can't run this at the moment because of an import error for babbage_fiscal:
https://github.com/openspending/os-api/blob/feature/dpp-runner/os_api/cube_manager.py#L2

Similarly, tests are failing because of a failed import for babbage_fiscal:
https://github.com/openspending/os-api/blob/feature/dpp-runner/tests/conftest.py#L5

@akariv akariv force-pushed the feature/dpp-runner branch from b9ea6f7 to 7d177af Compare March 25, 2018 12:24
@akariv akariv force-pushed the feature/dpp-runner branch 2 times, most recently from c8fcc2e to 66b8df8 Compare March 25, 2018 18:40
@akariv akariv force-pushed the feature/dpp-runner branch from 66b8df8 to 4ef014b Compare March 25, 2018 18:53
@akariv akariv force-pushed the feature/dpp-runner branch from 9ad0266 to c7712cc Compare March 25, 2018 19:26
@akariv akariv force-pushed the feature/dpp-runner branch from 759fe24 to 24918f6 Compare March 25, 2018 20:44
@akariv akariv force-pushed the feature/dpp-runner branch from 24918f6 to ad0aadc Compare March 26, 2018 04:29
@akariv akariv changed the title Bump babbage version Use dpp-runner instead of babbage-fiscal Apr 2, 2018
@akariv akariv force-pushed the feature/dpp-runner branch from a466bd1 to 3cfa552 Compare April 2, 2018 08:54
@akariv
Copy link
Copy Markdown
Member Author

akariv commented Apr 2, 2018

@brew tests are fixed now

@brew
Copy link
Copy Markdown
Contributor

brew commented Apr 3, 2018

Great!

I'm having a little trouble getting the tests to run locally, as there are some additional moving parts -- installing some system dependencies, running dpp and setting env var (e.g. DPP_DB_ENGINE). Can you add these additional steps and settings to the README in the testing section?

@akariv akariv force-pushed the feature/dpp-runner branch 2 times, most recently from aec8d29 to 5e8e6b2 Compare April 12, 2018 13:10
@akariv akariv force-pushed the feature/dpp-runner branch from 5e8e6b2 to 7eabd28 Compare April 12, 2018 13:11
@brew brew merged commit 7eabd28 into master Apr 16, 2018
@brew brew deleted the feature/dpp-runner branch April 17, 2018 14:53
brew added a commit to openspending/openspending that referenced this pull request Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants