OpenSubtitlesDownload icon indicating copy to clipboard operation
OpenSubtitlesDownload copied to clipboard

Load settings from JSON file instead of setting variables in .py file

Open Oreeeee opened this issue 2 years ago • 10 comments

Storing credentials in .py file is very unsafe since you can accidentally commit your password and loading from external file will make it easier to modify and update the script.

Oreeeee avatar May 13 '22 14:05 Oreeeee

Another reason for having the password in a config file is packages of this file, like https://aur.archlinux.org/packages/opensubtitlesdownload

You can't really edit a file that's part of a package. Well, it defeats the purpose of having a package that's going to be managed by the system

marco44 avatar Jul 09 '23 15:07 marco44

Then maybe adding the file to a different location like ~/.config/OpenSubtitlesDownload/config.json for Unix and %UserProfile%\Documents\OpenSubtitlesDownload\config.json would be a better idea?

Oreeeee avatar Jul 09 '23 15:07 Oreeeee

Oh, currently it's in the current working directory ? Yeah it would make much more sense I think…

marco44 avatar Jul 09 '23 16:07 marco44

I misunderstood the comment you made. I thought you were saying that the issue was that the config file was in current directory, and not that it is currently stored in the code. I made this PR over a year ago and I didn't remember the context. Anyways, I'm gonna move the config to the locations I mentioned.

Oreeeee avatar Jul 09 '23 17:07 Oreeeee

It should be now changed.

Oreeeee avatar Jul 09 '23 17:07 Oreeeee

I'm not a dev on this project. I was just stating that as another user, I would like this feature too, it's not practical passing the user/pass each time :)

marco44 avatar Jul 09 '23 19:07 marco44

Would this force someone to use the json file for credentials or would be optional for those who want to use it?

Kyshman avatar Feb 04 '24 10:02 Kyshman

@Kyshman This forces the user to use the JSON.

Oreeeee avatar Feb 04 '24 20:02 Oreeeee

Actually, looks like v6.2 includes a much better way of handling downloads for free users, shipping a hard-coded API key, and logging in being optional for paid users. But I won't close the PR, as the settings are still hard-coded.

Oreeeee avatar Feb 04 '24 21:02 Oreeeee

Hi, sorry I never answered this pull request, I didn't put much work in this software for the past couple of years...

@Kyshman This forces the user to use the JSON.

The problem is indeed that this change fundamentally how the software works. If the settings from the JSON file were just superseding the settings from the Python file, it would have been acceptable. The settings file path selection is also not very solid.

I have no problem letting this PR open however, if people want to use it or improve upon it.

emericg avatar Mar 27 '24 17:03 emericg