-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add the option '--if-not-exists' in the command 'ongr:es:index:create' #535
Changes from 2 commits
3a6b7cd
f4bacdc
f0164e1
2ff9e79
6c5c0ff
79cfbfa
7aad99f
230c997
a1bbef5
e509614
0ce9f2f
7543250
e74bfd9
f9ca1d6
4c6e91f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,13 @@ protected function configure() | |
InputOption::VALUE_NONE, | ||
'If the time suffix is used, its nice to create an alias to the configured index name.' | ||
) | ||
->addOption('no-mapping', 'nm', InputOption::VALUE_NONE, 'Do not include mapping'); | ||
->addOption('no-mapping', 'nm', InputOption::VALUE_NONE, 'Do not include mapping') | ||
->addOption( | ||
'if-not-exists', | ||
'ine', | ||
InputOption::VALUE_NONE, | ||
'Don\'t trigger an error, when the index already exists' | ||
); | ||
} | ||
|
||
/** | ||
|
@@ -55,6 +61,17 @@ protected function execute(InputInterface $input, OutputInterface $output) | |
$finder->setNextFreeIndex($manager); | ||
} | ||
|
||
if ($input->getOption('if-not-exists') && $manager->indexExists()) { | ||
$output->writeln( | ||
sprintf( | ||
'<info>Index `<comment>%s</comment>` already exists in `<comment>%s</comment>` manager.</info>', | ||
$manager->getIndexName(), | ||
$input->getOption('manager') | ||
) | ||
); | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure 100% but probably better would be change to: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct. As Doctrine's code about this processing, it will be
whatever success or error. Hence, here, we agree that 0 will be for fail and 1 for success. Is it OK? @saimaz |
||
} | ||
|
||
$manager->createIndex($input->getOption('no-mapping')); | ||
|
||
$output->writeln( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,37 @@ public function testExecute($managerName, $arguments, $options) | |
$manager->dropIndex(); | ||
} | ||
|
||
/** | ||
* Tests creating index in case of existing this index. Configuration from tests yaml. | ||
* | ||
* @param string $managerName | ||
* @param array $arguments | ||
* @param array $options | ||
* | ||
* @dataProvider getTestExecuteData | ||
*/ | ||
public function testExecuteWithIndexExistence($managerName, $arguments, $options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should not use different test case data provider and try to behave inside test case to match expectations. To test |
||
{ | ||
$manager = $this->getManager($managerName); | ||
|
||
if (!$manager->indexExists()) { | ||
$manager->createIndex(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Index is automatically created when you call |
||
} | ||
|
||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you will write test case, we do not need exception catch here. |
||
$arguments['--if-not-exists'] = null; | ||
$this->runIndexCreateCommand($managerName, $arguments, $options); | ||
} catch (\Exception $ex) { | ||
$message = $ex->getMessage(); | ||
$expectedClassName = 'Elasticsearch\\Common\\Exceptions\\BadRequest400Exception'; | ||
$isExpectedException = $ex instanceof $expectedClassName; | ||
if ($isExpectedException) { | ||
$this->assertNotContains('IndexAlreadyExistsException', $message); | ||
} | ||
} | ||
$manager->dropIndex(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created indexes are automatically removed after each test case. |
||
} | ||
|
||
/** | ||
* Tests if right exception is thrown when manager is read only. | ||
* | ||
|
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.
I think that Symfony does not support multi-letter input option shortcuts. I see that we have the same with other options
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.
http://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html
With this standard, it could understand wrong the options.