[Pending] Utils: DownLoadImage failed when PageRequest need Header "User-Agent"

seco

Development Group
  • Team MediaPortal
  • August 7, 2007
    1,580
    113
    Home Country
    Finland Finland
    Country flag
    Looking at the DownLoadImage code it is really a mess, I'm pretty sure there is a way more simple way to do what the current code does.
     

    morfius

    Portal Pro
    November 10, 2011
    40
    8
    53
    Valladolid
    Home Country
    Spain Spain
    Looking at the DownLoadImage code it is really a mess, I'm pretty sure there is a way more simple way to do what the current code does.
    I hope you are talking about OLD code. New code is very simple and really functional. I prefer to talk about the new code and possible improvements. ;)
     

    morfius

    Portal Pro
    November 10, 2011
    40
    8
    53
    Valladolid
    Home Country
    Spain Spain
    A bit of history. I detected the existing problem by chance, since MP1 could not download the covers from FilmAffinity. I reported the problem (i believe) in the right place (this post).
    After detecting the problem, i found the procedure in the code where the error occurred: the old routine (both entry points).
    I'm newbie to Media Portal and one do not enter in a house saying "Petite fudge!" code you have here.

    If you read the first post you will see that indicated three possible solutions. Both respectful with all members.
    At this point and after a lot of testing I came to the conclusion that, adding those two headers the problem was solved without modifying more than two lines of code.

    Note that those two lines the problem is solved. And as always when one sees an easy and elegant solution, think Why did not I think of that to me?

    Then try to go a step further by proposing a solution that changed the whole routine.
    I proposed a new routine with its program of testing to isolate as much as possible the check the same.

    Synchronous downloads is always a source of problems. We are talking about hundreds of images downloaded from websites that do not get excited every time a MediaPortal user decide to upgrade your collection of 1000 movies.

    But asynchronous download involves changing the internal structure of MediaPortal1
    (A brief comment. solutions by DownloadAsync I've tried over and over again, are not convincing.
    dotNET 4.5 implements Await is Stylish and Effective solution, at least in the tests I've done. But MP1 uses version 4.0 of the Framework.)

    I think the 26 lines of code that i have proposed are the proper lines, there is not a more or less, (excepion of Sleep. For this line i have alternatives.). We talk between developers. This 26 lines are the result of hundreds of lines, that involves my database and 100 images downloaded every check)

    After all that said, i would love to participate, collaborate with the team of Media Portal, as two lines or as the entire procedure. At the end, the ocean is just a set of water drops. And i would to contribute with my little water droplet. ;)

    Greetings from Spain!
     

    Sebastiii

    Development Group
  • Team MediaPortal
  • November 12, 2007
    16,350
    113
    France
    Home Country
    France France
    Country flag
    It's always good that new blood are around :)

    Recently we are lack of dev and tester, so it's really nice that you add addition and even more if you can :)

    The principle of how mp handle release is that when the fix or feature (adopted by the team) is ready and tested, the fix/feature goes to the next release -> That mean, a fix has not to wait to long time :)
     

    seco

    Development Group
  • Team MediaPortal
  • August 7, 2007
    1,580
    113
    Home Country
    Finland Finland
    Country flag
    Looking at the DownLoadImage code it is really a mess, I'm pretty sure there is a way more simple way to do what the current code does.
    I hope you are talking about OLD code. New code is very simple and really functional. I prefer to talk about the new code and possible improvements. ;)
    Unfortunately both old and proposed version are way too complex and low-level. You can do exactly same by using WebClient.DownloadFile and setting the correct headers. If you want to do it asynchronously you can say ThreadPool.QueueUserWorkItem(o => Download(file)).

    I've used all these in my plugin without any problems. No need to re-invent wheel when there are abstractions available in framework that should be used.
     

    morfius

    Portal Pro
    November 10, 2011
    40
    8
    53
    Valladolid
    Home Country
    Spain Spain
    Unfortunately both old and proposed version are way too complex and low-level. You can do exactly same by using WebClient.DownloadFile and setting the correct headers. If
    I suggest google search webclient vs httpwebrequest. There are a lot of experiences, especially in codeproject, web reference for c# programmers

    From Microsoft DOC:
    WebClientHeaders Property
    Some common headers are considered restricted and are protected by the system and cannot be set or changed in a WebHeaderCollection object. Any attempt to set one of these restricted headers in the WebHeaderCollection object associated with a WebClient object will throw an exception later when attempting to send the WebClient request.

    Restricted headers protected by the system include, but are not limited to the following:

    Date

    Host

    In addition, some other headers are also restricted when using a WebClient object. These restricted headers include, but are not limited to the following:

    Accept
    ..
    So, you can't use Header Accept
    ..
    If you want to do it asynchronously you can say ThreadPool.QueueUserWorkItem(o => Download(file)).
    This post is not about Syn or Async

    Really, i don't want to start in Media Portal in the wrong way or defend the indefensible. But i think my code is OK (except maybe Sleep)


    Greetings from Spain!
     

    seco

    Development Group
  • Team MediaPortal
  • August 7, 2007
    1,580
    113
    Home Country
    Finland Finland
    Country flag
    So, you can't use Header Accept
    Then I would suggest to try

    Code:
    using (Stream input = request.GetResponse().GetResponseStream())
    {
        using (Stream output = new FileStream(strFile, FileMode.OpenOrCreate, FileAccess.Write))
        {
            input.CopyTo(output);
        }
    }
    Ps. You should use { } consistently, also with using / streams

    Code:
    if (foo)
    {
    ..code..
    }
    Really, i don't want to start in Media Portal in the wrong way or defend the indefensible. But i think my code is OK (except maybe Sleep)
    No need to defend anything. If you want to become a better programmer feedback from code reviews / reading other people's code is one of the best ways to learn.
     

    morfius

    Portal Pro
    November 10, 2011
    40
    8
    53
    Valladolid
    Home Country
    Spain Spain
    Hi seco (and all). Thx for your comments.. I will rewrite the code with {}. But Sleep, what about Sleep and Catch?
     

    seco

    Development Group
  • Team MediaPortal
  • August 7, 2007
    1,580
    113
    Home Country
    Finland Finland
    Country flag
    Hi seco (and all). Thx for your comments.. I will rewrite the code with {}. But Sleep, what about Sleep and Catch?
    Error handling & retrying is not responsibility of the Download method and should be done by method caller as seen fit
     

    Users Who Are Viewing This Thread (Users: 0, Guests: 1)

    Top Bottom