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

FORCE NOT NULL and FORCE NULL support #45

Merged
merged 4 commits into from Aug 4, 2017
Merged

FORCE NOT NULL and FORCE NULL support #45

merged 4 commits into from Aug 4, 2017

Conversation

rdmurphy
Copy link
Contributor

@rdmurphy rdmurphy commented Aug 4, 2017

This resolves #44 and adds support for force_not_null and force_null options to CopyMapping.

I followed the same format as the other extra_options β€” but these two are a little bit more complicated because they need to support the passing of lists. Let me know if you think it should be a bit more robust!

I was able to reuse one of the other test fixtures for force_null, but none of the mocks fit what force_not_null is trying to accomplish, so I created a new mock and CSV for it.

I also think I correctly updated the documentation too, but I have little experience with Sphinx so I was kind of winging it. 😬

I've been testing this against an internal app and it solved my original issue. πŸŽ‰

@rdmurphy
Copy link
Contributor Author

rdmurphy commented Aug 4, 2017

I have no idea how Travis CI decides Postgres needs to be available (at least according the docs, you'd have to pass something in to services, but this project doesn't), but if I were to hazard a guess at why this failed the tests it's because it's testing against a version of Postgres that's too old to have support for FORCE NULL. That doesn't show up until Postgres 9.4. Travis CI apparently defaults to Postgres 9.1 if you don't specify.

@palewire
Copy link
Owner

palewire commented Aug 4, 2017

I've just added the following to the Travis config file in your fork. Let's see how that does.

addons:
 postgresql: "9.4"

@coveralls
Copy link

coveralls commented Aug 4, 2017

Coverage Status

Coverage decreased (-5.6%) to 94.423% when pulling cdd951a on rdmurphy:force-null-support into 6ab0b87 on california-civic-data-coalition:master.

@palewire palewire merged commit bc54d84 into palewire:master Aug 4, 2017
@palewire
Copy link
Owner

palewire commented Aug 4, 2017

Looks like it's passing now that I made that change. Let's merge it.

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

Successfully merging this pull request may close these issues.

Support FORCE NOT NULL and FORCE NULL
3 participants