[EMAIL PROTECTED] wrote: [snip] >> http://cr.opensolaris.org/~johansen/pkg-mirror1/src/depot.py.sdiff.html >> It might be worth checking the flags set to make sure they're sane, for >> example a --mirror with --refresh-refresh index wouldn't make sense. >> (the cr for 2693 that's out now has a 3 of these) and maybe we should >> abstract them into a check_sanity function, but don't worry about that. >> Anyway, I think checking the matches of --mirror with the other flags >> would probably be helpful. >> > > Would you provide a little more detail about the combinations that you > don't think are sane? These flags have appeared as I've merged with > different incoming stuff, and nobody has a sanity check yet. I'd be > interested to know what combinations you think don't make sense. > > For example, if mirroring is a more restricted version of read-only, then --mirror --refresh-index wouldn't make sense, nor would --mirror --rebuild. There will be some examples of this just as soon as the bug fix for 2693 gets pushed back. The idea is, if you know at start-up the options passed don't make sense, error then rather than waiting a while, and erroring when you don't have permissions to update the index (for example). >> http://cr.opensolaris.org/~johansen/pkg-mirror1/src/modules/client/filelist.py.sdiff.html >> line 226: I think in the future, we might want to treat these two errors >> differently, I'd be much more tolerant of a depot that timesout than one >> that actively gives me bad data. Definitely a future change, just >> thought I'd mention it. Though, on 234, we might want to distinguish >> between getting a time out from repeatedly getting bad data, both to the >> user and to our actions. Maybe if we get bad data, we just keep trying >> other mirrors until we have no more mirrors to try? Not sure that's the >> right the behavior either, but there's probably a happy medium somewhere. >> > > It sounds like you may be getting confused about the exceptions. That's > probably my fault. The verify code throws an InvalidContentException > that the filelist code currently doesn't catch. I was under the > impression that a corrupt file was a showstopper, but maybe not. > > A TransferContentException gets thrown when we get a read error off of > the tar stream. This happens when a depot can't send us any of the > files we asked for, so they instead send us an empty tar stream. We > catch the read error, raise this exception, and then try another mirror. > > Ah, when I said "treat these two errors differently" I meant penalize the depot differently, ie, add a different number of errors to their totals. But, you're right, I also didn't understand the exceptions. I'd argue that eventually, we might not want an InvalidContentException to be a show stopper and that instead we'd remove the corrupted file and try again from a different mirror, but perhaps not.
[snip] >> http://cr.opensolaris.org/~johansen/pkg-mirror1/src/modules/server/repository.py.sdiff.html >> line 156: might we want to log (or debug log when that comes in) the >> unsupported actions here? It might simplify things when people are >> going, "why isn't my depot supporting search?" (Or perhaps log a list of >> the supported requests instead?) >> > > This seems like it would be a potential for a DOS, resource exhaustion, > or at a minimum a logfile full of unhelpful errors. > > Fair enough, then how about something during start up with says in the server log (once) "Starting as mirror, only x, y and z are supported operations" or "a,b,c,d,e are not supported". > > Thanks for reviewing this. > > -j > > Glad to help, Brock _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
