python-plexapi icon indicating copy to clipboard operation
python-plexapi copied to clipboard

Initialize headers from config in reset_base_headers()

Open glensc opened this issue 4 years ago • 6 comments

WIP: https://github.com/pkkid/python-plexapi/issues/706

glensc avatar Mar 21 '21 18:03 glensc

OK, here's some idea that could work. Don't mind the PR title and body, will update them once the PR comes useful.

So, WDYT, is this acceptable?

glensc avatar Mar 22 '21 22:03 glensc

How about this approach? If needed, I could extract to smaller changes.

glensc avatar Apr 04 '21 20:04 glensc

I still don’t see the point. IMO you need to add a example on why this is better. Fix your flake errors, run the test suit locally, including the tests for the client and squash the commits.

I don’t like the name reset_default_headers as isn’t anymore, is the headers is in config file (it was a bad name in the first place) Why is the headers a local variable the class? Why not use it directly?

Hellowlol avatar Apr 04 '21 22:04 Hellowlol

Fix your flake errors, run the test suit locally, including the tests for the client and squash the commits.

I don't see point fixing tests until it's certain this is worth finishing! That's why I asked about the changes, not the style! Also, do note the PR is in a Draft state.

glensc avatar Apr 05 '21 07:04 glensc

I don’t like the name reset_default_headers as isn’t anymore, is the headers is in config file (it was a bad name in the first place)

I used an existing name, please suggest a better name not only complain it's bad.

  • get_base_headers?
  • get_plex_headers?

glensc avatar Apr 05 '21 07:04 glensc

Why is the headers a local variable the class? Why not use it directly?

I think I explained it in an issue this PR is referring to (https://github.com/pkkid/python-plexapi/issues/706). to make the headers instance-based, not global. so the self._headers is now copied into the instance. is that headers you are referring to?

since Config object may be modified runtime (after __init__) using copy initialized in __init__.py is the original problem.

ps: since Github has no threading in comment mode, submit a review as code comments, so can reply in a thread and resolve disucssions.

glensc avatar Apr 05 '21 07:04 glensc