phpcr / phpcr-utils

A set of helper classes and command line commands you want to use with phpcr, but that are not part of the API itself
phpcr.github.io
Other
72 stars 30 forks source link

Add option to not process existing workspace as error #137

Closed webda2l closed 10 years ago

webda2l commented 10 years ago

silent-on-exist or better if you prefer

dbu commented 10 years ago

@dantleech wdyt? i agree its a good idea to do something here.

we might just output a warning but return state 0, for automatic deployments. or a -f / --force parameter, like with rm?

dantleech commented 10 years ago

Hmm. I tend to do the following in these situations:

php app/console doctrine:phpcr:workspace create foobar &> /dev/null

But yeah, would be safer with a flag. Maybe it would be better to talk about idempotency: --idempotent ? Are there any similar flags in other commands in the world?

btw. Doctrine does not support this with database:create.

dbu commented 10 years ago

the rm command uses -f - when you force, it does not complain if the target file does not exist. from the use case of this command, i would either do -f or even just stop doing a non-0 exit state and have a --fail-if-exists option.

dantleech commented 10 years ago

But force in the case of rm is to force the removal. In this case it doesn't make sense, unless we are "forcing" the command not to throw an exception.

I think "idempotent" represents what we are doing well. If it doesn't exist, create it, if it does exist, do not do anything. This could also apply to other operations like workspace:remove, node:create etc.

webda2l commented 10 years ago

As you want. indempotent is not very common, but..! I use "&> /dev/null" tips currently, thanks.

dbu commented 10 years ago

Idempotent is very technical. What about ignore-existing (short i)? Or just print a warning but keep 0 exit state without any option?

dantleech commented 10 years ago

I think --ignore-existing is good (but I think we should not have a shortcut (yet) -- it would likely clash with other shortcuts as we should use this option for all commands with similar requirements).

dbu commented 10 years ago

@dantleech but you think there is worth in keeping this returning a non 0 status at all?

dantleech commented 10 years ago

Yeah, I think we should return non zero if the --ignore-existing flag is not used.

Some examples of other things:

Mysqladmin:

$ mysqladmin create test; echo "Exit code: "$?                                                                                                                                                                                    node-validator a7494d2 ✗
mysqladmin: CREATE DATABASE failed; error: 'Can't create database 'test'; database exists'
Exit code: 255

Doctrine:

$ ./app/console doctrine:database:create; echo "Exit code: "$?                                                                                                                                                               docker 76b0051 ✗
Could not create database for connection named `dtlweb1`
An exception occurred while executing 'CREATE DATABASE `dtlweb1`':

SQLSTATE[HY000]: General error: 1007 Can't create database 'dtlweb1'; database exists
Exit code: 1

Postgres

psql -Upostgres -c "create database sulutest"; echo "Exist code: "$?                                                                                                                                                       docker 76b0051 ✗
ERROR:  database "sulutest" already exists
Exist code: 1
dbu commented 10 years ago

okay, convinced. i see the code is already updated. thanks a lot.

dbu commented 10 years ago

the build fail is a hhvm issue with a test taking more than 1 second which has nothing to do with this PR

webda2l commented 10 years ago

Thanks! As previously, a minor release will be nice.

dbu commented 10 years ago

i would love to fix the hhvm fail before the release. anybody got a quick idea how to tell phpunit that its ok if a test takes > 1 second?

dantleech commented 10 years ago

You can use the @small, @medium and @large annotations: https://phpunit.de/manual/current/en/appendixes.annotations.html#appendixes.annotations.medium

dbu commented 10 years ago

there you go, just tagged 1.2.2

thanks a lot!