Log initial link when using API

[OMEGARAZER]

Logs the initial link rather than the direct API link.

This will make checking if the link works through a regular browser easier in cases such as #755

[Serene-Arc]

I'm not sure that this is an improvement over simply going through Reddit. Since we have the submission ID, it's as simple as reddit.com/{id} and then the browser goes through the entire redirect process that the BDFR goes through. I think that's a more natural way to do it.

There's also a thought I had; would the logging be more useful to take place in that base download function or in the higher level exception catching? You're replacing the actual URL that failed, which might have useful information, with the referral URL, which isn't actually what's getting checked. I don't think that should be done. If the API link failed, then at that level, that's what should be logged.

[OMEGARAZER]

I'm not sure that this is an improvement over simply going through Reddit. Since we have the submission ID, it's as simple as reddit.com/{id} and then the browser goes through the entire redirect process that the BDFR goes through. I think that's a more natural way to do it.

This doesn't change at all with this, the ID is still logged and available the same as it was previously

You're replacing the actual URL that failed, which might have useful information, with the referral URL, which isn't actually what's getting checked. I don't think that should be done. If the API link failed, then at that level, that's what should be logged.

The API URLs that show up now don't include any of the headers or anything that are required for them to work anyway so it then requires the step you mentioned above which not everyone would know. It just replaces a link that is essentially unusable with the link that Reddit provides on its own. That can be used to easily verify if the resource is available without the extra step of going through Reddit to get to the resource. The direct API link, outside of debugging which if you're debugging at that level you're able to put that API link together on your own, just isn't useful for basic troubleshooting. That's my thought process behind it anyway, just simpler for basic troubleshooting and if you're able to use the API link you likely won't need the log to get it.

There's also a thought I had; would the logging be more useful to take place in that base download function or in the higher level exception catching?

I'll admit I'm not sure I follow on this one so I can't comment, I think the logging is probably fine where it is but also haven't put much thought into it.

[Serene-Arc]

I mean, that function is for downloading every single web resource that the BDFR comes across, besides those which come from praw. If you want to log the API link, then it shouldn't be in that function. It should be higher up in the download loop or downloader modules where it's actually called. What you're doing would change something that I think is too base level. I think the proof of that is that you only changed three modules, not all of them, to log an initial URL.

[OMEGARAZER]

If you want to log the API link, then it shouldn't be in that function.

The exact opposite, this is not logging the API link at all, it's logging the link that's given from Reddit.

...you only changed three modules, not all of them, to log an initial URL.

Only 3 modules use an API, hence the changes in them.

It essentially changes this:

DEBUG] - Using Imgur with url https://i.imgur.com/TE3vaW.jpg
ERROR] - Site Imgur failed to download submission l1q4n2: Server responded with 404 at https://api.imgur.com/3/image/TE3vaW

to this:

DEBUG] - Using Imgur with url https://i.imgur.com/TE3vaW.jpg
ERROR] - Site Imgur failed to download submission l1q4n2: Server responded with 404 at https://i.imgur.com/TE3vaW.jpg

But most times the first line won't be seen because it's debug logged and the user will only see the API link which is all but useless to them. I'm just trying to make troubleshooting easier. Up to you in the end though.

[Serene-Arc]

That's what I'm saying. Those three modules use an API but you're changing the function for all of them.

If you want to log the API link, then it shouldn't be in that function.

Sorry I meant if you wanted to log the changed link not the API link, then it would be.

The user debugging is not really my concern, and like I said, pretending that the link that failed is something other than it is is a recipe for disaster down the road. We didn't request https://i.imgur.com/TE3vaW.jpg. We requested https://api.imgur.com/3/image/TE3vaW and that's what failed, so that's what should be logged. If you'd like to include the other link then a debug logging message can be added in the modules that use an API.