search

scalyr / scalyr-tool

Command-line tool for accessing Scalyr services
Apache License 2.0
56 stars 37 forks source link

Add Python 3 compatibility #5

Closed sgrimm-sg closed 8 years ago

sgrimm-sg commented 8 years ago

The script didn't work under Python 3, but the incompatibilities were minor and easy to fix while retaining full Python 2 compatibility.

sgrimm-sg commented 8 years ago

(Note that this pull request includes the change from my other pull request to fix the calls to argparse.)

czerwingithub commented 8 years ago

Hi Steve -

Thanks very much for taking the time to put this together. Can I ask you to handle the case where "from future import print_function" fails? If you don't have time, I'll just leave your pull request open and do the fix myself sometime next week.

About the fix: We have to maintain Python compatibility for 2.4 and higher (because we have customers that are running old RHEL5 systems). It looks to me like the "print" function is only supported for 2.6 and higher. It should be pretty easy to define a second print function (for 2.4 and 2.5) that implements the necessary functionality. Call it something like "print_func". Then define print_func to be print for 2.6 and higher. Replace all your uses of print with print_func.

Anyway, thanks for the help.

sgrimm-sg commented 8 years ago

If you need to support Python 2.4, then yeah, that will need to be wrapped. I'll update the patch. The future thing was added in 2.6.

sgrimm-sg commented 8 years ago

Printing to stderr was the only special case; for regular stdout printing, print("x") works fine in Python 2 and 3. Unfortunately I don't have easy access to a Python 2.4 install to test against.

czerwingithub commented 8 years ago

I can perform the test for 2.4 based on your changes before I merge it. Maybe you just define a print_stderr function then?

-Steve

On Thu, Dec 3, 2015 at 5:27 PM, sgrimm-sg notifications@github.com wrote:

Printing to stderr was the only special case; for regular stdout printing, print("x") works fine in Python 2 and 3. Unfortunately I don't have easy access to a Python 2.4 install to test against.

— Reply to this email directly or view it on GitHub https://github.com/scalyr/scalyr-tool/pull/5#issuecomment-161845324.

sgrimm-sg commented 8 years ago

Exactly what I did! (The pull request is already updated, but I guess Github doesn't send notifications about that.)

czerwingithub commented 8 years ago

Ah, no I didn't get the notification. Ok, thanks very much. I'll test it tomorrow morning and do the merge then.

-Steve

On Thu, Dec 3, 2015 at 5:31 PM, sgrimm-sg notifications@github.com wrote:

Exactly what I did! (The pull request is already updated, but I guess Github doesn't send notifications about that.)

— Reply to this email directly or view it on GitHub https://github.com/scalyr/scalyr-tool/pull/5#issuecomment-161845763.

czerwingithub commented 8 years ago

Hello -

Sorry for the delay on this. We had a customer issue on Friday that took most of my attention. Anyway, I did test on 2.4 -- we require you to install a few extra modules from what 2.4 normally provides.. but all was good after one small syntax fix.

Your changes are merged into the master branch. Thanks for the pull request.

-Steve

On Thu, Dec 3, 2015 at 5:31 PM, Steven Czerwinski czerwin@scalyr.com wrote:

Ah, no I didn't get the notification. Ok, thanks very much. I'll test it tomorrow morning and do the merge then.

-Steve

On Thu, Dec 3, 2015 at 5:31 PM, sgrimm-sg notifications@github.com wrote:

Exactly what I did! (The pull request is already updated, but I guess Github doesn't send notifications about that.)

— Reply to this email directly or view it on GitHub https://github.com/scalyr/scalyr-tool/pull/5#issuecomment-161845763.