Re: Summary of store_url project and some questions before posting some patches.
lör 2012-10-06 klockan 17:24 +0200 skrev Eliezer Croitoru: > Code implementation is quite simple but since I was working with the old > bzr revision 12317 ant now its about 30 revisions up I dont have a clue > on what to do. are you stuck on repository format upgrade, or on incompatible changes in later revisions of squid?
Re[2]: Summary of store_url project and some questions before posting some patches.
> Original Message >From: Alex Rousskov >To: "Eliezer Croitoru" >Cc: "Squid Developers" >Sent: Sat, Oct 6, 2012, 11:49 PM >Subject: Re: Summary of store_url project and some questions before posting >some patches. > >On 10/06/2012 09:24 AM, Eliezer Croitoru wrote: >> As I moved forward and managed to make store_url feature stable to pass >> all of my tests The next step is to state a summary of the feature. >> >> The goal of store_url feature is to give squid a way to handle >> De-duplication of objects. >> >> Code implementation is quite simple but since I was working with the old >> bzr revision 12317 ant now its about 30 revisions up I dont have a clue >> on what to do. >> for now I downloaded the new 2a revision 12349. What do you think about that? does it matters at all? >> >> >> How it was done in squid 2.7 is less relevant for squid 3 branch >> implementation and was reviewed in the other thread. >> >> We discussed earlier on "url" to "originalUrl" refactoring and it seems >> to me that this will make a big and unneeded modification to the code >> that is not needed to achieve the goal. > >One reason to rename the old "url" field is so that we can double check >that you caught all usage cases. If you do not rename, the patch will >not show the cases you missed (if any). Any renaming changes for this >reason alone can be later dropped (after reviews and before the commit). > >Another reason is to alert future developers that they are dealing with >a URL which may be different from the store URL. If this is a valid/good >reason, the renaming should stay. Ok seems reasonable to me. > > >> Also At every place the store_url is mentioned can be the indication of >> it while every other place a "url" or "canonicalUrl" is mentioned will >> be the mark of original url usage. > >Yes, but those "every other places" will not be visible in the patch and >since "url" is a rather generic term some of them may be difficult to >find in Squid sources using a manual search. > I wont say that there is no possiblitiy but I didnt used only manual search for it since it's unreasonable in such a huge ammount of code. I indeed found one place that I needed to change the code and since it was in a #IF statement only the compiler was able to recognize it. > >HTH, > >Alex. Thanks, Helps a lot!
Re: Summary of store_url project and some questions before posting some patches.
On 10/06/2012 09:24 AM, Eliezer Croitoru wrote: > As I moved forward and managed to make store_url feature stable to pass > all of my tests The next step is to state a summary of the feature. > > The goal of store_url feature is to give squid a way to handle > De-duplication of objects. > > Code implementation is quite simple but since I was working with the old > bzr revision 12317 ant now its about 30 revisions up I dont have a clue > on what to do. > for now I downloaded the new 2a revision 12349. > > > How it was done in squid 2.7 is less relevant for squid 3 branch > implementation and was reviewed in the other thread. > > We discussed earlier on "url" to "originalUrl" refactoring and it seems > to me that this will make a big and unneeded modification to the code > that is not needed to achieve the goal. One reason to rename the old "url" field is so that we can double check that you caught all usage cases. If you do not rename, the patch will not show the cases you missed (if any). Any renaming changes for this reason alone can be later dropped (after reviews and before the commit). Another reason is to alert future developers that they are dealing with a URL which may be different from the store URL. If this is a valid/good reason, the renaming should stay. > Also At every place the store_url is mentioned can be the indication of > it while every other place a "url" or "canonicalUrl" is mentioned will > be the mark of original url usage. Yes, but those "every other places" will not be visible in the patch and since "url" is a rather generic term some of them may be difficult to find in Squid sources using a manual search. HTH, Alex.
Summary of store_url project and some questions before posting some patches.
As I moved forward and managed to make store_url feature stable to pass all of my tests The next step is to state a summary of the feature. The goal of store_url feature is to give squid a way to handle De-duplication of objects. Code implementation is quite simple but since I was working with the old bzr revision 12317 ant now its about 30 revisions up I dont have a clue on what to do. for now I downloaded the new 2a revision 12349. How it was done in squid 2.7 is less relevant for squid 3 branch implementation and was reviewed in the other thread. We discussed earlier on "url" to "originalUrl" refactoring and it seems to me that this will make a big and unneeded modification to the code that is not needed to achieve the goal. Also At every place the store_url is mentioned can be the indication of it while every other place a "url" or "canonicalUrl" is mentioned will be the mark of original url usage. The steps we were talking about in the feature implementation are: 1. add the "fake" store_url interface to the helper based on redirect. 2. prepare all methods\functions and variables to allow the actual store_url happen. 3. add the actual implementation of store_url feature (making fake into real) and adding the needed documentation and configuration related code to allow the usage of the feature. 4. integrating store_url related code into PURGE,ICP,HTCP. Things to consider? Proposals for Tests that should be done are more then welcome. Eliezer -- Eliezer Croitoru https://www1.ngtech.co.il IT consulting for Nonprofit organizations eliezer ngtech.co.il
