Skip to content

[3.0.x.x] replaced wget with curl ecb controller#13642

Merged
AJenbo merged 2 commits intoopencart:3.0.x.xfrom
condor2:patch-4
Feb 5, 2024
Merged

[3.0.x.x] replaced wget with curl ecb controller#13642
AJenbo merged 2 commits intoopencart:3.0.x.xfrom
condor2:patch-4

Conversation

@condor2
Copy link
Copy Markdown
Contributor

@condor2 condor2 commented Feb 3, 2024

No description provided.

@AJenbo AJenbo merged commit bad5aa3 into opencart:3.0.x.x Feb 5, 2024
@condor2 condor2 deleted the patch-4 branch February 5, 2024 07:47
@mhcwebdesign
Copy link
Copy Markdown
Contributor

Why? There was no bug!

@AJenbo
Copy link
Copy Markdown
Contributor

AJenbo commented Feb 5, 2024

Not all serves include wget by default, but curl is part of the standard package set.

@condor2
Copy link
Copy Markdown
Contributor Author

condor2 commented Feb 5, 2024

Why? There was no bug!

You have tested currency update using hosting Cron?

@mhcwebdesign
Copy link
Copy Markdown
Contributor

Why? There was no bug!

You have tested currency update using hosting Cron?

Yes, works fine on our webhost.

@condor2
Copy link
Copy Markdown
Contributor Author

condor2 commented Feb 5, 2024

With?

wget -q "ecb_url" >/dev/null 2>&1

@AJenbo
Copy link
Copy Markdown
Contributor

AJenbo commented Feb 5, 2024

@condor2 try saying what error you get when you do that...

@condor2
Copy link
Copy Markdown
Contributor Author

condor2 commented Feb 5, 2024

@condor2 try saying what error you get when you do that...

Using wget will download a bunch of 0 kb files on server.

index.php?route=extension%2Fcurrency%2Fecb%2Frefresh
index.php?route=extension%2Fcurrency%2Fecb%2Frefresh.1
index.php?route=extension%2Fcurrency%2Fecb%2Frefresh.2

@AJenbo
Copy link
Copy Markdown
Contributor

AJenbo commented Feb 5, 2024

So that is 2 good reasons to switch to curl in favor of wget.

  • wget can leave clutter on the server
  • wget is not available on all server configurations

@mhcwebdesign
Copy link
Copy Markdown
Contributor

mhcwebdesign commented Feb 5, 2024

You are quick coming to the wrong conclusion. The original call was a simple

wget -q "https://www.your-domain.com/index.php?route=extension/currency/ecb/refresh"

for the CRON task, and it works perfectly fine, without generated zero-bytes files.

What webhost doesn't have wget ? This one of the most common Linux/Unix commands available on webhosts.

If you want to submit pull-requests for bugfixes, you are welcome, but please do some proper testing first, or at least provide some explanations.

@condor2
Copy link
Copy Markdown
Contributor Author

condor2 commented Feb 5, 2024

You are quick coming to the wrong conclusion. The original call was a simple

wget -q "https://www.your-domain.com/index.php?route=extension/currency/ecb/refresh"

I have tested before. On my server download/generate zero-bytes files. so thats why I have tried with >/dev/null 2>&1, but same.

So I use curl -s without zero-bytes files, but feel free to change back to wget -q

@mhcwebdesign
Copy link
Copy Markdown
Contributor

According to wget --help:

GNU Wget 1.20.3, a non-interactive network retriever.
Usage: wget [OPTION]... [URL]...
....
-q,  --quiet                     quiet (no output)
....

So ask your webhost to fix your wget then, we have run this command on countless servers/webhosts, never had this issue.

condor2 added a commit to condor2/opencart that referenced this pull request Feb 5, 2024
@AJenbo
Copy link
Copy Markdown
Contributor

AJenbo commented Feb 5, 2024

You are quick coming to the wrong conclusion.

You are quick to dismiss a problem with out understanding it.

The original call was a simple wget -q

Now it's curl -s, I don't see how one is simpler then the other, they are the exact same number and type of symbols.

wget -q "https://www.your-domain.com/index.php?route=extension/currency/ecb/refresh"

for the CRON task, and it works perfectly fine, without generated zero-bytes files.

Which vendor is your wget from? It could be a different between for example the BusyBox and GNU implementation.

What webhost doesn't have wget ? This one of the most common Linux/Unix commands available on webhosts.

Lots. curl is part of the ubuntu-server meta package, wget is not, minimalistic setups based on ubuntu-server would be setup like that.

please do some proper testing first

What is it that wasn't tested here?

@AJenbo
Copy link
Copy Markdown
Contributor

AJenbo commented Feb 5, 2024

According to wget --help:

GNU Wget 1.20.3, a non-interactive network retriever.
Usage: wget [OPTION]... [URL]...
....
-q,  --quiet                     quiet (no output)
....

So ask your webhost to fix your wget then, we have run this command on countless servers/webhosts, never had this issue.

You are misunderstanding that description, "quiet (no output)" means nothing is printed on stdout, not that it will not generate a file. This is used to avoid clutter in the server logs, not to have it not save the downloaded file. I'm starting to think you are the one that didn't test this.

To prevent wget from saving a file you need to give it something wget -O /dev/null URL.

@mhcwebdesign
Copy link
Copy Markdown
Contributor

@AlenJo: OK, you right, the wget should indeed use the -q --no-check-certificate -O /dev/null . I stand corrected here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants