-
-
Notifications
You must be signed in to change notification settings - Fork 151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SSL certificate API. #59
Conversation
…ule from which there are.
Thank you @ajarzyn and great work, although there is gonna be some break changes in peoples code due to naming changes it is a good clean up and optimization, the thing I don't understand is why not include base_api_core into general auth without changing names, in that way the updated library won't break codes and won't be multiple auth modules, I think would be better. could you consider just adding module adding the base_api_core into general auth? |
Hello @N4S4 I did not consider others people code at all, so thank you for pointing that out. Idea behind base_api_core is that lot's of modules could base on Core but not all of them, and all of them will use Authentication but with different object, thus my proposition to implement it like this. Authentication does not depend on Core but Core depend on Authentication, just like every other "application". Could you draft me a use case where change would brake others code? As I understand base_api_core would not be imported by user but rather by API chunks like Certificate and others. I could add a wrapper for ActiveBackup, Backup and sys_info, to ensure back compatibility, but I would suggest to put warning, that this object is depreciated and ask users to perform, proper changes. From the other hand, it's python, changes can be quite dynamic right? Please check this, here we can see that there are many different libraries for SYNO.Core, but also you can see other "applications" like SYNO.DSM, SYNO.Backup, and many more, thus I think it would be good to prepare this API for use case like this. PS. Different aspect I wanted to talk about is making this API more flexible. If you have any proposition please do not hositate to write them out. Thanks. |
It actually make sense, I was thinking just of the issue of renamed files would break code of somebody using it, but yes it is python. Improvements are always welcomed. Honestly I never investigated about other synology products (as I cared and worked on my NAS) and would be interesting to implement other products API too as long there is going to be an available tester on that specific product. What products are you using? I'll test your implementation and in that case I'll write in the change log the breaking changes for renamed files. At first look seems all good just give me a bit of time. |
Hi sorry for long delay, it's hard to find spare moment. I would like to propose refactoring/redesigning this module, to prepare it for what it may happen. This pull request shows that project structure could be better, and what is more important, there is no clear interface designed on which we could base new functionality. I use RT2600ac and MR2200ac, but only RT2600ac is of interest here, as it's used as a main gate for all network. Regarding file name changes we actually can make it step by step, implementing SSL functionality only, and create new branch with refactoring new interface could be better idea. Making many different changes would cursed by all the good people using your module. |
It's ok I dont have much time too due to my job, |
Hello,
I've added few methods to from SSL certificate API.
I couldn't edit README (don't know why) thus example of usage here:
This code import certificates to NAS, and make new imported cert as default.
I've also added parent object for core functionality not to duplicate to much code.
Because of that I edited files name for better indication of what application they are from.
Hope you will like changes.
Tested:
Certificate
SysInfo.storage()