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

refactor(db) a more consistent options handling #3772

Merged
merged 1 commit into from Sep 15, 2018

Conversation

Projects
None yet
3 participants
@bungle
Copy link
Member

bungle commented Sep 15, 2018

Summary

Refactored kong.db.dao to be more consistent with the options table.

This was originally proposed in
#3710

And this is one of the series of PRs that were previously implemented in that branch.

@kikito

kikito approved these changes Sep 15, 2018

@kikito

kikito approved these changes Sep 15, 2018

@@ -138,11 +145,11 @@ end
-- including entries that have been since overriden, and those
-- with weight=0 (i.e. the "raw" representation of targets in
-- the database)
function _TARGETS:select_by_upstream_raw(upstream_pk)
function _TARGETS:select_by_upstream_raw(upstream_pk, ...)

This comment has been minimized.

Copy link
@kikito

kikito Sep 15, 2018

Member

I think the ... can be replaced with the param options here.

@kikito

This comment has been minimized.

Copy link
Member

kikito commented Sep 15, 2018

@bungle I don't know how or why, but this seems to be triggering a "memory corruption" problem on nginx on travis. I have tried re-running the specs, but the problem persists.

Two possible things that could fix this (I am not sure):

  • maybe replace the varargs (...) with options as I mentioned before
  • try not localizing all the functions? Or localizing them in the scope they are used, instead of at the top of the file (shooting blind here)
@bungle

This comment has been minimized.

Copy link
Member Author

bungle commented Sep 15, 2018

@kikito memory corruption problem is unrelated to this PR. It is a problem in the Travis OpenSSL builds triggered by OpenSSL / luaossl bump (@thibaultcha resolved that, and I think the fix will land to next soon).

@bungle bungle force-pushed the refactor/db-options branch 3 times, most recently from 70298c8 to ab5b272 Sep 15, 2018

Show resolved Hide resolved kong/db/dao/targets.lua Outdated

@bungle bungle force-pushed the refactor/db-options branch 2 times, most recently from dd620c1 to 0eb0f06 Sep 15, 2018

@bungle bungle force-pushed the refactor/db-options branch from 0eb0f06 to b9afe3b Sep 15, 2018

@bungle bungle merged commit 26192ec into next Sep 15, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@thibaultcha thibaultcha deleted the refactor/db-options branch Sep 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.