[AtlasDB Proxy & DbKVS] Part 7: Allow Namespace Overrides for Oracle Connection Config

[jeremyk-91]

Goals (and why):
Pay down some legacy technical debt, and make configuration in atlasdb-proxy look smoother.
==COMMIT_MSG==
Users may now explicitly override the namespace of an Oracle connection configuration, which is usually the mechanism by which an individual AtlasDB user identifies itself (e.g. to timelock and possibly to the underlying key-value-service). This was previously automatically assumed to be the sid of the Oracle database; however, that is not actually the way an individual AtlasDB user identifies itself.
==COMMIT_MSG==

Implementation Description (bullets):

• Allow explicit overriding of namespace while defaulting to the old behaviour; see above for discussion.

Testing (What was existing testing like? What have you done to improve it?):

• Added tests for the new behaviour.

Concerns (what feedback would you like?):

• This might be bad if someone has explicitly a useless override namespace: X in their config, though in the worst case it'll just mean that they throw as opposed to doing anything explicitly wrong.

Where should we start reviewing?: small

Priority (whenever / two weeks / yesterday): this week please

[changelog-app]

Generate changelog in changelog/@unreleased

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

Users may now explicitly override the namespace of a DB key value service configuration, which is usually the mechanism by which an individual AtlasDB user identifies itself (e.g. to timelock and possibly to the underlying key-value-service). This was previously automatically assumed to be the sid of the Oracle database or dbname of the Postgres database; however, that is not actually the way an individual AtlasDB user identifies itself in practice.

Check the box to generate changelog(s)

• Generate changelog entry

[gsheasby]

Why would we throw if a useless namespace: X is specified? It looks like we would silently use X so far, although of course if there's some validation rules about namespace names, then it could end up being invalidated.

[gsheasby]

this is clever, but a bit arcane - worth a javadoc here explaining the order of precedence.

[jeremyk-91]

yep, will add

[gsheasby]

JsonProperty("namespace")? Or do we want oracleNamespace, to explicitly be different from the existing/C* config?

[jeremyk-91]

Yep, no harm doing that. I think I went with override for explicit precedence order, though it's a fair argument saying it's implicit.

[gsheasby]

do we want to log (at least at debug, to match other logs in this class) if namespaceOverride is present?

Similarly, noting that check enforces exactly one of sid and serviceNameConfiguration to be present, which may not be needed any more (although I don't have enough context to know if these params are required elsewhere).

[jeremyk-91]

I think we still want the check - the purpose of namespace is mainly to outline how we identify ourselves to timelock (notice that we don't actually reference this field anywhere else in the DbKVS code). It doesn't actually see any use in connecting to the db

... in which case it probably makes sense for this not to be here.

[jeremyk-91]

Fair point on useless namespace: X - that is an artifact of the original version of the PR before I decided to pursue a different approach to avoid that risk altogether. I'll reply to the remainder of the comments on-thread

[jeremyk-91]

discussed offline: On reflection, I think this should belong in the general DbKVS config as opposed to in the connection config, seeing as it doesn't actually influence the connections to the underlying database (as much as that's where it's computed in the current, incorrect model).

[svc-autorelease]

Released 0.575.0