Joentje / nordvpn-proxy

Use NordVPN in your Docker stack
141 stars 53 forks source link

Update get-ovpn-files.sh #73

Closed mbryde closed 3 years ago

mbryde commented 3 years ago

This will hopefully fix some of the issues with trying to redownloading the OVPN files over and over again. Now it should only do it when:

  1. the file don't already exsist.
  2. if the current file is older than two hours. NOTE: This has not been tested directly yet.

Should hopefully close these issues: Close #70 Close #49 Close #42

gerwim commented 3 years ago

NOTE: This has not been tested directly yet.

I have merged it into my fork, will test it the coming days ;-).

gerwim commented 3 years ago

Three minor issues, see: https://github.com/gerwim/nordvpn-proxy/commit/5c83b41c8a01c32eec8f2f9d5e78a731865a6a23

Joentje commented 3 years ago

@MBryde nice contribution 👍 did you have a reason for the 2 hours?

mbryde commented 3 years ago

@MBryde nice contribution 👍 did you have a reason for the 2 hours?

No not really. Could be less or more. Or even a variable you could set but default too something. It more or less up too you as the owner of the repo.

mbryde commented 3 years ago

Three minor issues, see: gerwim@5c83b41

I have changed the suggestions you made. Thank you for pointing it out. I'm kind of new to bash

mbryde commented 3 years ago

@MBryde nice contribution 👍 did you have a reason for the 2 hours?

I made it a variable you can change. But defaults to 120 minutes.

mbryde commented 3 years ago

@gerwim Did you have time to test it yet?

gerwim commented 3 years ago

@MBryde I did. Just restarted the container, which comes back with:

/app/ovpn/get-ovpn-files.sh: line 9: download_files: command not found

I didn't debug the issue yet.

mbryde commented 3 years ago

@MBryde I did. Just restarted the container, which comes back with:

/app/ovpn/get-ovpn-files.sh: line 9: download_files: command not found

I didn't debug the issue yet.

Just read up on it a bit. And it's because the function is called before it's declared. Will move the function up before it's called. That should hopefully fix it.

mbryde commented 3 years ago

I have tested it myself. And it seems too work just fine. It won't re-download the files unless they either don't exists or they are older than the specified time. @Joentje if you could please test it now and close the RP that would be great.

Joentje commented 3 years ago

Seems to be working fine! Great contribution! Thanks @gerwim @mbryde