search

web2py / py4web

Other
261 stars 128 forks source link

fix for py-37 #807

Closed ali96343 closed 1 year ago

ali96343 commented 1 year ago

Minor code improvements. Added an example of writing to the server-py4web.log from the controller. Tested with rocket,wsgiref, tornado, gevent Fix: encoding="utf-8" for python <= 3.9

Sukhichev commented 1 year ago
  1. I'm not sure that a writing to the server-py4web.log from the controller is good idea, because server-py4web.log is apps server log file not apps log file. In other words, this is not public API. Your app can write the logs to its own log file.
  2. Your example has a global variable. Please read this: https://www.readersfact.com/are-global-variables-bad-python/
  3. Also you add two global "constants" to the code. It doesn't look good either.
  4. You don't fix bug. Really you change one bug to other bug, because in you case encoding=None for python < 3.9. I.e., log file will write in a platform-dependent encoding that is different for different platforms.
  5. Your commit contains many issues. It is really hard to read. Could you split your commit to some commits where one commit is one issue?
ali96343 commented 1 year ago

Hello

1 For debugging event-driven programs (i.e. Socketio, Tornado, SSE), it is convenient to see the sequence and times of events in a single log file.

2.3 We can see global variables in almost any python library file

  1. My patch allows py4web to work with python version >=3.7. I think that's enough

5 Here is my working version server_adapters.py.

https://github.com/ali96343/lvsio/blob/main/mig1ssl/server_adapters.py

I am ready to answer any questions about this file

Thank you for your comments