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

namespace: support set namespace for meta #834

Merged
merged 2 commits into from Nov 8, 2017

Conversation

Projects
None yet
3 participants
@disksing
Member

disksing commented Nov 6, 2017

Fix #824

@disksing disksing requested review from dantin, siddontang and nolouch Nov 6, 2017

c.Assert(tableClassifier.AddMetaToNamespace("test1"), IsNil)
// Can't be set again.
c.Assert(tableClassifier.AddMetaToNamespace("test1"), NotNil)
c.Assert(tableClassifier.AddMetaToNamespace("test2"), NotNil)

This comment has been minimized.

@nolouch

nolouch Nov 7, 2017

Member

why we just allow one namespace to add meta?

@nolouch

nolouch Nov 7, 2017

Member

why we just allow one namespace to add meta?

This comment has been minimized.

@disksing

disksing Nov 7, 2017

Member

Same behavior as tables, a table or meta is only allow to bind to a namespace.

@disksing

disksing Nov 7, 2017

Member

Same behavior as tables, a table or meta is only allow to bind to a namespace.

return namespace.DefaultNamespace
}
for name, ns := range c.nsInfo.namespaces {
_, ok := ns.TableIDs[tableID]
if ok {
if ok || (isMeta && ns.Meta) {
return name

This comment has been minimized.

@siddontang

siddontang Nov 7, 2017

Member

any test to cover meta here?

@siddontang

siddontang Nov 7, 2017

Member

any test to cover meta here?

@siddontang

LGTM

@siddontang siddontang merged commit a685b9b into pingcap:master Nov 8, 2017

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-pd/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@disksing disksing deleted the disksing:meta-namespace branch Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment