search

N4S4 / synology-api

A Python wrapper around Synology API
MIT License
384 stars 142 forks source link

Certificate api new feature, fixes and refactoring. #63

Closed ajarzyn closed 3 years ago

ajarzyn commented 3 years ago

Hello,

Sorry for the bug I've created with certificate upload (it did not verified if there were ssl verification on or of). Thus I'm submitting new changes with fix, some refactoring, and clean up.

I've noticed that some files in your repository have different line endings , thus I also wanted to fix with other commit.

Although in commit 046fd65 file auth.py is all glowy, there are only few real changes:

  1. Added def verify_cert_enabled
  2. from requests.packages.urllib3.exceptions import InsecureRequestWarning
  3. requests.packages.urllib3.disable_warnings(InsecureRequestWarning) sorry for messy commit I did not realized that file ending were different.

How you will like it. Best of luck AJ

ajarzyn commented 3 years ago

Oh and I almost forgot. Here you can see working script example: https://gist.github.com/ajarzyn/260ed4f44c0f43919281b7379efd15cb

N4S4 commented 3 years ago

Yeah i just notice this morning and i was starting to work on it today and thank you for the fix Just a small favor, when upload changes to an existing file to try to modify only the single line, otherwise win github will appeat that have been changed thousands of line

thank you

ajarzyn commented 3 years ago

It was to late with auth.py as I didn't noticed that I've changed line brake encoding, sorry for that.

But if you check commit "Fix and sync line separators to LF (Unix)." there are changes only for line brakes change.

Or maybe you would like me to create separate merge request for different changes? For example:

  1. for feature fix
  2. for adding new APIs
  3. for line bakes

?

N4S4 commented 3 years ago

If it does not bother you it would be the best otherwise will be messy for me to trace back changes on github and I have to compare all the new and old everything there is a pr

Thank you for ur support btw

ajarzyn commented 3 years ago

I'm sorry but I'm still missing what is your idea right there. If you are saying that I shall create different pull request for different features implemented, then reluctantly I could do that. I can image that there will be big change with many different fixes and commits and splitting it into separated merge per commit base would be really annoying a hard to not loose track of my work.

Anyway I though that most changes is tracked by commits. If you look at commit list there are two commits from last merge: Fix path for downloaded cert. Refactoring. Fix for ssl verify. Added … - this one I've asked for forgiveness as I accidentally changed line break character. NO WHITE SPACE DIFF: Fix path for downloaded cert. Refactoring. Fix for ssl verify. Added … Fix and sync line separators to LF (Unix). - but here are only line brake changes, no other changes.

N4S4 commented 3 years ago

No it is ok, maybe I didn't explain my self, All changes are tracked, It was just a friendly reminder not to change the entire file that's it. A single PR for all features is more than fine also because it would be hard for me too to deal with many PR. So no worries