-
Notifications
You must be signed in to change notification settings - Fork 204
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
Minor fixes for local execution #464
Conversation
mantis.taskexecutor.blob-store.storage-dir=file:///localstore_dir | ||
mantis.taskexecutor.blob-store.local-cache=/apps/mantis/mantis-server-agent/mantis-artifacts | ||
mantis.taskexecutor.blob-store.storage-dir=file:///Users/[email protected]/Downloads/ | ||
mantis.taskexecutor.blob-store.local-cache=/tmp/mantis/mantis-artifacts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewrite this.
run { | ||
standardInput = System.in | ||
// standardInput = System.in | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get rid of this block completely
Test Results126 files - 1 126 suites - 1 6m 46s ⏱️ +32s For more details on these failures, see this check. Results for commit 7e6ca0a. ± Comparison against base commit b78be8a. This pull request removes 2 tests.
♻️ This comment has been updated with latest results. |
@@ -668,6 +668,7 @@ private HttpClient<ByteBuf, ServerSentEvent> getRxnettySseClient(String hostname | |||
|
|||
private WebSocketClient<TextWebSocketFrame, TextWebSocketFrame> getRxnettyWebSocketClient(String host, | |||
int port, String uri) { | |||
logger.info("Creating websocket client for " + host + ":" + port + " uri " + uri + " ..."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get rid of this change
import io.mantisrx.master.events.LifecycleEventPublisher; | ||
import io.mantisrx.server.master.store.KeyValueStore; | ||
|
||
public class InMemoryPersistenceProvider extends KeyValueBasedPersistenceProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to tests.
return null; | ||
} | ||
|
||
@Override | ||
public ResourceClusterSpecWritable getResourceClusterSpecWritable(ClusterID id) | ||
throws IOException { | ||
return null; | ||
} | ||
|
||
@Override | ||
public ResourceClusterScaleRulesWritable getResourceClusterScaleRules(ClusterID clusterId) | ||
throws IOException { | ||
return null; | ||
} | ||
|
||
@Override | ||
public ResourceClusterScaleRulesWritable registerResourceClusterScaleRule( | ||
ResourceClusterScaleRulesWritable ruleSpec) throws IOException { | ||
return null; | ||
} | ||
|
||
@Override | ||
public ResourceClusterScaleRulesWritable registerResourceClusterScaleRule( | ||
ResourceClusterScaleSpec rule) throws IOException { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be filled
@@ -89,15 +90,16 @@ public class ResourceClusterNonLeaderRedirectRouteTest extends JUnitRouteTest { | |||
private final ActorSystem system = | |||
ActorSystem.create(ResourceClusterNonLeaderRedirectRouteTest.class.getSimpleName()); | |||
|
|||
private final IMantisPersistenceProvider storageProvider = new FileBasedPersistenceProvider(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to InMemoryPersistenceProvider
import org.junit.BeforeClass; | ||
import org.junit.Test; | ||
|
||
public class ResourceClusterAwareSchedulerActorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this file
when(resProvider.getResponseHandler()).thenReturn(responseHandler); | ||
|
||
ActorRef resourceClusterActor = system.actorOf( | ||
ResourceClustersHostManagerActor.props(resProvider, resStorageProvider)); | ||
ResourceClustersHostManagerActor.props(resProvider, new FileBasedPersistenceProvider(false))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace this with something else.
class InMemoryStore implements KeyValueStore { | ||
|
||
// table -> partitionKey -> secondaryKey -> data | ||
private final Map<String, Map<String, Map<String, String>>> store = new ConcurrentHashMap<>(); | ||
|
||
@Override | ||
public List<String> getAllPartitionKeys(String tableName) throws IOException { | ||
try { | ||
return store.get(tableName).keySet().stream().collect(Collectors.toList()); | ||
} catch (Exception e) { | ||
throw new IOException(e); | ||
} | ||
} | ||
|
||
@Override | ||
public Map<String, String> getAll(String tableName, String partitionKey) | ||
throws IOException { | ||
if (store.get(tableName) == null) { | ||
return Collections.emptyMap(); | ||
} else if (store.get(tableName).get(partitionKey) == null) { | ||
return Collections.emptyMap(); | ||
} else { | ||
return store.get(tableName).get(partitionKey); | ||
} | ||
} | ||
|
||
@Override | ||
public boolean upsertAll(String tableName, String partitionKey, Map<String, String> all, | ||
Duration ttl) throws IOException { | ||
store.putIfAbsent(tableName, new ConcurrentHashMap<>()); | ||
store.get(tableName).put(partitionKey, new ConcurrentHashMap<>(all)); | ||
return true; | ||
} | ||
|
||
@Override | ||
public boolean delete(String tableName, String partitionKey, String secondaryKey) | ||
throws IOException { | ||
if (store.containsKey(tableName) && // table exists | ||
store.get(tableName).containsKey(partitionKey) && // partitionKey exists | ||
store.get(tableName).get(partitionKey).containsKey(secondaryKey)) { // secondaryKey exists | ||
store.get(tableName).get(partitionKey).remove(secondaryKey); | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
@Override | ||
public boolean deleteAll(String tableName, String partitionKey) throws IOException { | ||
if (store.containsKey(tableName) && // table exists | ||
store.get(tableName).containsKey(partitionKey)) { // partitionKey exists | ||
store.get(tableName).remove(partitionKey); | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hmit Can you check if the semantics of this is consistent with the KeyValueStore assumptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thanks for this change!
1f769bf
to
b1610fe
Compare
Context
Explain context and other details for this pull request.
Checklist
./gradlew build
compiles code correctly./gradlew test
passes all tests