valepakh commented on code in PR #897: URL: https://github.com/apache/ignite-3/pull/897#discussion_r906100770
########## modules/cli/src/main/java/org/apache/ignite/cli/commands/cliconfig/CliConfigActivateCommand.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.cli.commands.cliconfig; + +import jakarta.inject.Inject; +import java.util.concurrent.Callable; +import org.apache.ignite.cli.call.cliconfig.CliConfigActivateCall; +import org.apache.ignite.cli.commands.BaseCommand; +import org.apache.ignite.cli.core.call.CallExecutionPipeline; +import org.apache.ignite.cli.core.call.StringCallInput; +import picocli.CommandLine.Command; +import picocli.CommandLine.Parameters; + +/** + * Command for activate profile as current. + */ +@Command(name = "activate") Review Comment: It would be nice to add a description for new commands and their parameters. ########## modules/cli/src/main/java/org/apache/ignite/cli/commands/cliconfig/CliConfigSubCommand.java: ########## @@ -24,26 +24,31 @@ import org.apache.ignite.cli.commands.BaseCommand; import org.apache.ignite.cli.commands.decorators.ConfigDecorator; import org.apache.ignite.cli.core.call.CallExecutionPipeline; -import org.apache.ignite.cli.core.call.EmptyCallInput; +import org.apache.ignite.cli.core.call.StringCallInput; import picocli.CommandLine.Command; +import picocli.CommandLine.Option; /** * Parent command for CLI configuration commands. */ @Command(name = "config", subcommands = { CliConfigGetSubCommand.class, - CliConfigSetSubCommand.class + CliConfigSetSubCommand.class, + CliConfigCreateProfileCommand.class Review Comment: activate command is not in the subcommands? ########## modules/cli/src/main/java/org/apache/ignite/cli/commands/cliconfig/CliConfigCreateProfileCommand.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.cli.commands.cliconfig; + +import jakarta.inject.Inject; +import java.util.concurrent.Callable; +import org.apache.ignite.cli.call.cliconfig.CliConfigCreateProfileCall; +import org.apache.ignite.cli.call.cliconfig.CliConfigCreateProfileCallInput; +import org.apache.ignite.cli.commands.BaseCommand; +import org.apache.ignite.cli.core.call.CallExecutionPipeline; +import picocli.CommandLine; + +/** + * Command for create CLI profile. + */ [email protected](name = "create-profile") +public class CliConfigCreateProfileCommand extends BaseCommand implements Callable<Integer> { + @CommandLine.Option(names = {"--name", "-n"}, required = true, description = "Name of new profile.") + private String profileName; + + @CommandLine.Option(names = {"--copy-from", "-c"}, description = "Profile whose content will be copied to new one.") + private String copyFrom; + + @CommandLine.Option(names = {"--activate", "-a"}, description = "Activate new profile as current or not.") + private boolean activate; Review Comment: ```suggestion @Command(name = "create-profile") public class CliConfigCreateProfileCommand extends BaseCommand implements Callable<Integer> { @Option(names = {"--name", "-n"}, required = true, description = "Name of new profile.") private String profileName; @Option(names = {"--copy-from", "-c"}, description = "Profile whose content will be copied to new one.") private String copyFrom; @Option(names = {"--activate", "-a"}, description = "Activate new profile as current or not.") private boolean activate; ``` ########## modules/cli/src/main/java/org/apache/ignite/cli/core/exception/handler/ConfigStoringExceptionHandler.java: ########## @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.cli.core.exception.handler; + +import org.apache.ignite.cli.config.ConfigStoringException; +import org.apache.ignite.cli.core.exception.ExceptionHandler; +import org.apache.ignite.cli.core.exception.ExceptionWriter; +import org.apache.ignite.lang.IgniteLogger; + +/** + * Handler for {@link ConfigStoringException}. + */ +public class ConfigStoringExceptionHandler implements ExceptionHandler<ConfigStoringException> { + private static final IgniteLogger log = IgniteLogger.forClass(ConfigStoringExceptionHandler.class); + + @Override + public int handle(ExceptionWriter err, ConfigStoringException e) { + log.error("CLI config storing error: ", e); + err.write("Error happen while saving CLI config " + e.getMessage()); Review Comment: ```suggestion err.write("Error happened while saving CLI config " + e.getMessage()); ``` ########## modules/cli/src/main/java/org/apache/ignite/cli/config/ini/IniConfigManager.java: ########## @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.cli.config.ini; + +import static org.apache.ignite.cli.config.ConfigConstants.CLUSTER_URL; +import static org.apache.ignite.cli.config.ConfigConstants.CURRENT_PROFILE; +import static org.apache.ignite.cli.config.ConfigConstants.JDBC_URL; + +import java.io.File; +import java.io.IOException; +import java.util.NoSuchElementException; +import org.apache.ignite.cli.config.ConfigConstants; +import org.apache.ignite.cli.config.ConfigInitializationException; +import org.apache.ignite.cli.config.ConfigManager; +import org.apache.ignite.cli.config.Profile; +import org.apache.ignite.cli.config.ProfileNotFoundException; + +/** + * Implementation of {@link ConfigManager} based on {@link IniFile}. + */ +public class IniConfigManager implements ConfigManager { + private static final String INTERNAL_SECTION_NAME = "ignitecli_internal"; + + private final IniFile configFile; + + private String currentProfileName; + + /** + * Constructor. + * + * @param file ini file. + */ + public IniConfigManager(File file) { + IniFile configFile; + try { + configFile = new IniFile(file); + findCurrentProfile(configFile); + } catch (IOException | NoSuchElementException e) { + configFile = createDefaultConfig(); + } + this.configFile = configFile; + this.currentProfileName = findCurrentProfile(configFile).getProperty(CURRENT_PROFILE); + } + + private static IniSection findCurrentProfile(IniFile configFile) { + IniSection internalSection = configFile.getSection(INTERNAL_SECTION_NAME); + if (internalSection == null) { + IniSection section = configFile.getSections().stream().findFirst().orElseThrow(); + internalSection = configFile.createSection(INTERNAL_SECTION_NAME); + internalSection.setProperty(CURRENT_PROFILE, section.getName()); + } + return internalSection; + } + + @Override + public Profile getCurrentConfig() { + IniSection section = configFile.getSection(currentProfileName); + if (section == null) { + throw new ProfileNotFoundException(INTERNAL_SECTION_NAME); + } + return new IniProfile(section, configFile::store); Review Comment: ```suggestion return getConfig(currentProfileName); ``` ########## modules/cli/src/main/java/org/apache/ignite/cli/config/ini/SectionAlreadyExistException.java: ########## @@ -0,0 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.cli.config.ini; + +/** + * Exception when already created INI section trying to create. + */ +public class SectionAlreadyExistException extends RuntimeException { + public SectionAlreadyExistException(String name) { + super("Section " + name + " already exist."); Review Comment: ```suggestion public class SectionAlreadyExistsException extends RuntimeException { public SectionAlreadyExistsException(String name) { super("Section " + name + " already exists."); ``` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
