----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/#review419 -----------------------------------------------------------

This is neither a full review nor a blocking issue, just an opportunity to make the code nicer.

src/rolekit/async.py (lines 61 - 62) http://reviewboard-fedoraserver.rhcloud.com/r/158/#comment239

With PEP 380, “yield async.call_future” should disappear in favor of “yield from”

src/rolekit/async.py (lines 102 - 103) http://reviewboard-fedoraserver.rhcloud.com/r/158/#comment238

What is the specific Python 3 requirement? 3.0?

This would be a simplification worth trying I think (carefully because removing the last yield would replace a generator with an ordinary function, but hiding this difference would also be nice).

- Miloslav Trmac

On Čer. 17, 2015, 5:44 odp., Stephen Gallagher wrote:

---

This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/

---

(Updated Čer. 17, 2015, 5:44 odp.)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description

Switch to Python 3

So far, only the Database Server Role and the main role infrastructure has been converted. I will port over the Domain Controller next. This is made visible for early review.

There is a lot of tracing added in this patch which aided in debugging the port. I decided not to separate it into a separate patch primarily because it would be too much effort.

Diffs

config/Makefile.am 8d6df0e54809cfa1858d9ecc955205a6d68478b5 config/roles/databaseserver/role.py b363d29cc1430ebd19924c6029386f059ad5183b config/roles/databaseserver/tools/rk_db_setpwd.py c7fecaaa78a392b2d0ea2525d3a563169d852c29 configure.ac dd4b473e5983ced7f3ecf08e457d4578d1eaa9f0 fix_python_shebang.sh 8a0430ce35605e0067f03fa2219c902a8bcbc85c rolekit.spec 8d798da1c386997061c19e393ea95a3940ff6db1 src/rolectl 7d978c2365bbb8e67b19df2d95bf6ac04b8cfe90 src/roled 954071f1d9ee3bedff3525728ba8d5a0c0ec49ed src/rolekit/async.py b2ba8f8f048757efcf57cb7eaf19fc071c28e5a6 src/rolekit/server/rolebase.py 5deceba3fa7f63f25ff9021c57a63b5470abc1f0

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/158/diff/

Testing

Deployed both Domain Controller roles and Database Server Roles to a fresh VM using Vagrant.

Thanks,

Stephen Gallagher

----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/ -----------------------------------------------------------

(Updated June 17, 2015, 4:19 p.m.)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Changes -------

Explicitly require python 3.4+

Repository: rolekit

Description -------

Switch to Python 3

So far, only the Database Server Role and the main role infrastructure has been converted. I will port over the Domain Controller next. This is made visible for early review.

There is a lot of tracing added in this patch which aided in debugging the port. I decided not to separate it into a separate patch primarily because it would be too much effort.

Diffs (updated) -----

config/Makefile.am 8d6df0e54809cfa1858d9ecc955205a6d68478b5 config/roles/databaseserver/role.py b363d29cc1430ebd19924c6029386f059ad5183b config/roles/databaseserver/tools/rk_db_setpwd.py c7fecaaa78a392b2d0ea2525d3a563169d852c29 configure.ac dd4b473e5983ced7f3ecf08e457d4578d1eaa9f0 fix_python_shebang.sh 8a0430ce35605e0067f03fa2219c902a8bcbc85c rolekit.spec 8d798da1c386997061c19e393ea95a3940ff6db1 src/rolectl 7d978c2365bbb8e67b19df2d95bf6ac04b8cfe90 src/roled 954071f1d9ee3bedff3525728ba8d5a0c0ec49ed src/rolekit/async.py b2ba8f8f048757efcf57cb7eaf19fc071c28e5a6 src/rolekit/server/rolebase.py 5deceba3fa7f63f25ff9021c57a63b5470abc1f0

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/158/diff/

Testing -------

Deployed both Domain Controller roles and Database Server Roles to a fresh VM using Vagrant.

Thanks,

Stephen Gallagher

----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/#review421 -----------------------------------------------------------

config/roles/databaseserver/tools/rk_db_setpwd.py (line 78) http://reviewboard-fedoraserver.rhcloud.com/r/158/#comment241

Should be noted that input is a different function on Python2 which equals to running eval on raw_input.

src/rolekit/async.py (line 133) http://reviewboard-fedoraserver.rhcloud.com/r/158/#comment242

Check `if exception is not None` and simple `if exception` is not the same thing.

In `if exception is not None` will go to else branch only if exception is None.

In `if exception` will got else branch if exception is None, or 0, or '', or whatever what evaluates to False

- Robert Kuska

On jún 24, 2015, 12:11 popoludní, Stephen Gallagher wrote:

---

This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/

---

(Updated jún 24, 2015, 12:11 popoludní)

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description

Switch to Python 3

So far, only the Database Server Role and the main role infrastructure has been converted.

There is a lot of tracing added in this patch which aided in debugging the port. I decided not to separate it into a separate patch primarily because it would be too much effort.

Diffs

config/Makefile.am 8d6df0e54809cfa1858d9ecc955205a6d68478b5 config/roles/databaseserver/role.py b363d29cc1430ebd19924c6029386f059ad5183b config/roles/databaseserver/tools/rk_db_setpwd.py c7fecaaa78a392b2d0ea2525d3a563169d852c29 configure.ac dd4b473e5983ced7f3ecf08e457d4578d1eaa9f0 fix_python_shebang.sh 8a0430ce35605e0067f03fa2219c902a8bcbc85c rolekit.spec 8d798da1c386997061c19e393ea95a3940ff6db1 src/rolectl 7d978c2365bbb8e67b19df2d95bf6ac04b8cfe90 src/roled 954071f1d9ee3bedff3525728ba8d5a0c0ec49ed src/rolekit/async.py b2ba8f8f048757efcf57cb7eaf19fc071c28e5a6 src/rolekit/server/rolebase.py 5deceba3fa7f63f25ff9021c57a63b5470abc1f0

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/158/diff/

Testing

Deployed both Domain Controller roles and Database Server Roles to a fresh VM using Vagrant.

Thanks,

Stephen Gallagher

----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/158/ -----------------------------------------------------------

(Updated July 9, 2015, 7:49 p.m.)

Status ------

This change has been marked as submitted.

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.

Repository: rolekit

Description -------

Switch to Python 3

So far, only the Database Server Role and the main role infrastructure has been converted.

There is a lot of tracing added in this patch which aided in debugging the port. I decided not to separate it into a separate patch primarily because it would be too much effort.

Diffs -----

config/Makefile.am 8d6df0e54809cfa1858d9ecc955205a6d68478b5 config/roles/databaseserver/role.py b363d29cc1430ebd19924c6029386f059ad5183b config/roles/databaseserver/tools/rk_db_setpwd.py c7fecaaa78a392b2d0ea2525d3a563169d852c29 configure.ac dd4b473e5983ced7f3ecf08e457d4578d1eaa9f0 fix_python_shebang.sh 8a0430ce35605e0067f03fa2219c902a8bcbc85c rolekit.spec 8d798da1c386997061c19e393ea95a3940ff6db1 src/rolectl 7d978c2365bbb8e67b19df2d95bf6ac04b8cfe90 src/roled 954071f1d9ee3bedff3525728ba8d5a0c0ec49ed src/rolekit/async.py b2ba8f8f048757efcf57cb7eaf19fc071c28e5a6 src/rolekit/server/rolebase.py 5deceba3fa7f63f25ff9021c57a63b5470abc1f0

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/158/diff/

Testing -------

Deployed both Domain Controller roles and Database Server Roles to a fresh VM using Vagrant.

Thanks,

Stephen Gallagher