dsmiley commented on a change in pull request #23:
URL: https://github.com/apache/solr/pull/23#discussion_r601875746
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -191,19 +213,79 @@ public void uploadConfig(Path dir, String configName)
throws IOException {
}
@Override
- public void uploadFileToConfig(String fileName, String configName) throws
IOException {
- Path path = Paths.get(fileName);
- if (!Files.exists(path)) {
- throw new IOException("File path " + path + " does not exist");
+ public void createFilePathInConfig(String configName, String fileName,
boolean failOnExists) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
failOnExists,true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file path in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data, boolean failOnExists) throws IOException {
+ String filePath = CONFIGS_ZKNODE + "/" + configName + "/" + fileName;
+ try {
+ zkClient.makePath(filePath, data, CreateMode.PERSISTENT, null,
failOnExists, true);
+ } catch (KeeperException.NodeExistsException nodeExistsException) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The path " + filePath + " for configSet " + configName + " already
exists. In order to overwrite, provide overwrite=true or use an HTTP PUT with
the V2 API.");
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, Map<Object, Object> data)
throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName, Utils.toJSON(data),
true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error setting config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, byte[] data) throws
IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName, data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error setting config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void updateConfigMetadata(String configName, Map<Object, Object>
data) throws IOException {
+ try {
+ zkClient.setData(CONFIGS_ZKNODE + "/" + configName, Utils.toJSON(data),
true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error updating config metadata",
SolrZkClient.checkInterrupted(e));
}
- File file = path.toFile();
- String zkNode = CONFIGS_ZKNODE + "/" + configName + "/" + file.getName();
+ }
+
+ @Override
+ public void updateConfigMetadata(String configName, byte[] data) throws
IOException {
+ try {
+ zkClient.setData(CONFIGS_ZKNODE + "/" + configName, data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error updating config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public byte[] getConfigMetadata(String configName) throws IOException {
Review comment:
Lets work with Maps and not byte arrays for metadata. Bytes are too low
level.
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -191,19 +213,79 @@ public void uploadConfig(Path dir, String configName)
throws IOException {
}
@Override
- public void uploadFileToConfig(String fileName, String configName) throws
IOException {
- Path path = Paths.get(fileName);
- if (!Files.exists(path)) {
- throw new IOException("File path " + path + " does not exist");
+ public void createFilePathInConfig(String configName, String fileName,
boolean failOnExists) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
failOnExists,true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file path in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data, boolean failOnExists) throws IOException {
+ String filePath = CONFIGS_ZKNODE + "/" + configName + "/" + fileName;
+ try {
+ zkClient.makePath(filePath, data, CreateMode.PERSISTENT, null,
failOnExists, true);
+ } catch (KeeperException.NodeExistsException nodeExistsException) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The path " + filePath + " for configSet " + configName + " already
exists. In order to overwrite, provide overwrite=true or use an HTTP PUT with
the V2 API.");
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, Map<Object, Object> data)
throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName, Utils.toJSON(data),
true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error setting config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, byte[] data) throws
IOException {
Review comment:
No; I think the API of ConfigSetService should keep this higher level
than bytes. The Map one is perfect.
##########
File path: solr/core/src/java/org/apache/solr/core/ConfigSetService.java
##########
@@ -246,23 +247,53 @@ protected NamedList loadConfigSetFlags(CoreDescriptor cd,
SolrResourceLoader loa
public abstract String configSetName(CoreDescriptor cd);
/**
- * Check whether a config exists
+ * Upload files from a given path to config
*
- * @param configName the config to check existance on
- * @return whether the config exists or not
- * @throws IOException if an I/O error occurs
+ * @param dir {@link java.nio.file.Path} to the files
+ * @param configName the name to give the config
+ * @throws IOException if an I/O error occurs or the path does not exist
*/
- public boolean checkConfigExists(String configName) throws IOException {
- return listConfigs().contains(configName);
- }
+ public abstract void uploadConfig(Path dir, String configName) throws
IOException;
Review comment:
Let's have a consistent/purposeful approach to parameter ordering in
this class. We could make "configName" first *always*. Or, we could vary it
to go last *only* when we uploading (e.g. imagine an arrow showing the
direction of the data from first param to second).
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
##########
@@ -781,6 +785,14 @@ public String getConfig() {
}
}
+ public List<String> getAllConfigsetFiles(String root) throws
KeeperException, InterruptedException {
Review comment:
This is needed on SolrZkClient for some reason?
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -191,19 +213,79 @@ public void uploadConfig(Path dir, String configName)
throws IOException {
}
@Override
- public void uploadFileToConfig(String fileName, String configName) throws
IOException {
- Path path = Paths.get(fileName);
- if (!Files.exists(path)) {
- throw new IOException("File path " + path + " does not exist");
+ public void createFilePathInConfig(String configName, String fileName,
boolean failOnExists) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
failOnExists,true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file path in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data, boolean failOnExists) throws IOException {
Review comment:
RE "failOnExists": Lets standardize on naming used by Files.write (see
StandardOpenOption) -- "CREATE_NEW"
##########
File path: solr/core/src/java/org/apache/solr/core/ConfigSetService.java
##########
@@ -285,27 +316,124 @@ public boolean checkConfigExists(String configName)
throws IOException {
public abstract void copyConfig(String fromConfig, String toConfig,
Set<String> copiedToZkPaths) throws IOException;
/**
- * Upload files from a given path to a config Zookeeper
+ * Create file path in config
*
- * @param dir {@link java.nio.file.Path} to the files
- * @param configName the name to give the config
- * @throws IOException if an I/O error occurs or the path does not exist
+ * @param configName the config to download
+ * @param fileName file path to be created
+ * @param failOnExists if file path already exists then do not re-create
+ * @throws IOException if an I/O error occurs
*/
- public abstract void uploadConfig(Path dir, String configName) throws
IOException;
+ public abstract void createFilePathInConfig(String configName, String
fileName, boolean failOnExists) throws IOException;
Review comment:
I'm skeptical this is necessary. Instead, files can be uploaded that
have slashes in there and we can create recursively as needed.
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -223,6 +315,42 @@ public void downloadConfig(String configName, Path dir)
throws IOException {
}
}
+ @Override
+ public List<String> listFilesInConfig(String configName) throws IOException {
+ try {
+ return zkClient.getChildren(CONFIGS_ZKNODE + "/" + configName, null,
true);
+ } catch (KeeperException.NoNodeException e) {
+ return Collections.emptyList();
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException(e);
+ }
+ }
+
+ @Override
+ public List<String> listFilesInConfig(String configName, String fileName)
throws IOException {
Review comment:
You added getAllConfigsetFiles which seems to obsolete this method?
Though I like the name of this one better.
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -191,19 +213,79 @@ public void uploadConfig(Path dir, String configName)
throws IOException {
}
@Override
- public void uploadFileToConfig(String fileName, String configName) throws
IOException {
- Path path = Paths.get(fileName);
- if (!Files.exists(path)) {
- throw new IOException("File path " + path + " does not exist");
+ public void createFilePathInConfig(String configName, String fileName,
boolean failOnExists) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
failOnExists,true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file path in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data) throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName + "/" + fileName,
data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void uploadFileToConfig(String configName, String fileName, byte[]
data, boolean failOnExists) throws IOException {
+ String filePath = CONFIGS_ZKNODE + "/" + configName + "/" + fileName;
+ try {
+ zkClient.makePath(filePath, data, CreateMode.PERSISTENT, null,
failOnExists, true);
+ } catch (KeeperException.NodeExistsException nodeExistsException) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The path " + filePath + " for configSet " + configName + " already
exists. In order to overwrite, provide overwrite=true or use an HTTP PUT with
the V2 API.");
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error creating file in config",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, Map<Object, Object> data)
throws IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName, Utils.toJSON(data),
true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error setting config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void setConfigMetadata(String configName, byte[] data) throws
IOException {
+ try {
+ zkClient.makePath(CONFIGS_ZKNODE + "/" + configName, data, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error setting config metadata",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void updateConfigMetadata(String configName, Map<Object, Object>
data) throws IOException {
Review comment:
I observe you added setConfigMetadata yet also have
updateConfigMetadata. They are the same! (a bug surely). I suspect we may
not need the "update" version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
