On 16.03.2016 15:13, Martin Basti wrote:
On 16.03.2016 14:59, Oleg Fayans wrote:
Hi Martin
On 03/16/2016 02:39 PM, Martin Basti wrote:
On 16.03.2016 10:59, Oleg Fayans wrote:
With this patch applied integration tests pass and in-tree tests are
gracefully skipped.
@mkubik, It is not possible to put the decorator to util.py as per our
discussion, because it uses tasks, so tasks must be imported. But
tasks
already import util, which leads to circular imports. So I've put
it to
tasks.py
NACK
1)
Use right ticket in commit message (#5723)
But (#5736) is exactly the issue that is being addressed. Probably note
both tickets in the commit message?
But as I wrote in ticket #5736, this ticket should be closed, because
issue is caused by ticket which is not finished yet, so we should
continue just with original ticket.
2)
Link to ticket should be last in the commit message
3)
dereplicafy
3a)
wrong doc string, it removes *only* replicas not clients
No, in fact it removes both:
uninstall_replica(args[0].master, host)
uninstall_client(host)
Both tasks have raiseonerr set to False, which means that even if
replica was not installed but the client was - it will also be removed
I see just
for host in args[0].replicas
I don't see any
for host in args[0].clients
there
Also uninstall_client should not be there. ipa-server-install
--uninstall removes client too. The extra call of uninstall client is
IMO there just because an ancient bug that is already fixed.
3b)
can we rename it to something different? (replicas_cleanup,
replicas_uninstall, replicas_teardown)
replicas_cleanup, or even topo_cleanup sounds OK to me.
4)
Please fix commit message
- Wile trated correctly
- followiong
- rewrote -> rewrite
Will do
5)
decorator
+ def wrapped(*args):
+ func(*args)
+ for host in args[0].replicas:
Shouldn't be there try-finally around func() call, or something?
No, the wrapped function is a test_* method: if it fails we need to see
the original failure
but if something raise an exception in func(), cleanup will not be
executed.
You can do
In [4]: try:
...: raise ValueError('Hello')
...: finally:
...: try:
...: raise ValueError('Cleanup')
...: except Exception:
...: pass
...:
---------------------------------------------------------------------------
ValueError Traceback (most recent call
last)
<ipython-input-4-affb927f7603> in <module>()
1 try:
----> 2 raise ValueError('Hello')
3 finally:
4 try:
5 raise ValueError('Cleanup')
ValueError: Hello
On the other hand, I do not want cleanup with --pdb option, so maybe it
should just fail
Are you sure that there is no need to return result of func()?
The same applies here: we never return results from test_* methods
ok
*) Please create additional patch that will add licence there
Will do :)
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
