Filter out leaderPaxos from list of namespaces #4760
Conversation
Generate changelog in
|
...ock-impl/src/test/java/com/palantir/atlasdb/timelock/management/DiskNamespaceLoaderTest.java
Outdated
Show resolved
Hide resolved
...ock-impl/src/test/java/com/palantir/atlasdb/timelock/management/DiskNamespaceLoaderTest.java
Outdated
Show resolved
Hide resolved
...ock-impl/src/test/java/com/palantir/atlasdb/timelock/management/DiskNamespaceLoaderTest.java
Outdated
Show resolved
Hide resolved
...ock-impl/src/test/java/com/palantir/atlasdb/timelock/management/DiskNamespaceLoaderTest.java
Outdated
Show resolved
Hide resolved
| @Rule | ||
| public TemporaryFolder tempFolder = new TemporaryFolder(); | ||
|
|
||
| private DiskNamespaceLoader diskNamespaceLoader; |
There was a problem hiding this comment.
nit: I believe the Rule can be made final, and with that you can make this final and set it here as opposed to inside setup() (the test as written works, this is just general cleanliness).
There was a problem hiding this comment.
The DiskNamespaceLoader requires path to the tempFolder which is only available after the folder has been created. The only other option TemporaryFolder api gives is to provide the path of parent file. There is not a way to determine the path of the temp folder before it is created (which happens in the before block).
| private static final String NAMESPACE_2 = "namespace_2"; | ||
|
|
||
|
|
||
| public final @Rule TemporaryFolder tempFolder = new TemporaryFolder(); |
There was a problem hiding this comment.
| public final @Rule TemporaryFolder tempFolder = new TemporaryFolder(); | |
| @Rule | |
| public final TemporaryFolder tempFolder = new TemporaryFolder(); |
This is fine also, just for consistency with the other tests.
| private static final String NAMESPACE_1 = "namespace_1"; | ||
| private static final String NAMESPACE_2 = "namespace_2"; | ||
|
|
||
|
|
|
|
||
| @Test | ||
| public void doesNotLoadLeaderPaxosAsNamespace() { | ||
| Set<String> namespaces = diskNamespaceLoader.getAllPersistedNamespaces().stream().map(client -> client.value()).collect( |
There was a problem hiding this comment.
nit: Let's use a method reference here (Client::value as opposed to x -> x.value())
| } | ||
|
|
||
| private void createDirectoryForLeaderForEachClientUseCase(String namespace) { | ||
| if (Paths.get(tempFolder.getRoot().toPath().toString(), |
There was a problem hiding this comment.
nit: I think it's probably nicer to do tempFolder.getRoot().toPath().resolve(LEADER_PAXOS_NAMESPACE).resolve(...) which should behave in the same way
Goals (and why):
leaderPaxos is not a namespace itself. It should be filtered out when loading namespaces from disk.
Implementation Description (bullets):
Filter out "leaderPaxos" while loading namespaces
Testing (What was existing testing like? What have you done to improve it?):
None
Concerns (what feedback would you like?):
None
Where should we start reviewing?:
DiskNamespaceLoader.java
Priority (whenever / two weeks / yesterday):
Anytime