Re: [PATCH]proposal for a fake_strore_url helper wrapping.

On 09/10/2012 05:39 PM, Alex Rousskov wrote:
> On 09/09/2012 04:08 AM, Eliezer Croitoru wrote:
>> I took the url_rewrite helper and used what ever seems needed to create
>> a url_store_rewrite helper.
>> I also added the needed variables for the next step and freeing them
>> where and if needed.
>> this is a more of a basic sketch.
> Please do not mark sketch patches not meant for commit with a [PATCH]
> prefix. [RFC] or no prefix would be better.
OK
> When you think these changes are ready for commit, please include a
> proposed commit message. It is a good idea to do that even before the
> last stage, of course -- it tells us what the patch is supposed to do.
> "what ever seems needed to create a url_store_rewrite helper" is too
> vague. If the code is committed with such a message, most of the
> knowledge gained during recent discussions will be lost and somebody
> would have to start from scratch when they want to understand how the
> new feature works and, more importantly, why it should work that way.
it's not that vague if you do ask me.
the process is pretty simple if you do ask me (after reading XK++ lines)
I dont want to maintain a branch but to push it into squid as a feature.
I proposed from the beginning to make it in stages:
1. embed a fake store url rewrite fake.(naming is not really important)
as grounds to the next steps.
this part is pretty important by itself because no one even done that in
the 3+.
2. embed the code that actually handles the data from the external
program when storing it in squid process.
3. use it at every point needed in squid while handling the requests.

>
> Please do not forget to document all new members (using doxygen
> comments) -- this is one of the mandatory requirements for patches. And,
> as Amos pointed out, remove "for storeurl" marks -- everything you
> change in this patch is for the storeurl feature (or at least it should
> be) so the diff blobs themselves can be used as such a mark.
Since I Have never used doxygen and friends in my code before I will try
to learn them on the way.

>
> If this code is not sufficient to support the storeurl feature, I do not
> think it should be committed to trunk -- it has no stand-alone value.
> Commit it to your own branch so that you can maintain the progress
> history and post incremental patches for discussion. When the feature is
> completed, your branch can then be merged into trunk.
I do have a plan since From all the data I gathered and code review I
did until now it's pretty straight forward.

I will look at the guidelines of squid3 code (started already)

So you do remind me that I didnt proposed *yet* the basic idea which is
critical.
what I wanted is to get all the patches ready and then commit them.
I must state I'm testing my code and not waiting until it breaks something.

If you do want to know exactly what is being done or what I propose:
get the rewritten url from the external software.
put it in a char * variable (while NULLED at initialization).
the access to it will be using the mem_obj->(name:strore_uri,storeurl..
whatever)
the only really important place I have seen that any code should be
committed in other parts then redirect and friends is at the
getpublickey and friends.
The above is since squid only finds HIT by hash(has was discussed before).

So the way to do it is pretty simple like it was before:
while accessing any place mem_obj->url (needed for has and friends) it
will transform into:
if (mem_obj->store__uri\url){
  use store_uri\url in operation
}
else {
use uri\url in operation

}

if you\someone need me to explain anything I'm here for it.

Doing it the *right *way?
we are deciding if it's the right way so I will propose a working way
that seems efficient enough and that will work.

and a question:
since the http object has uri,log_uri we used store_uri to match the others.
but at the mem-obj there is a url methods.
so do we want to stick with anything on the way? nameing the method at
mem_obj->storeurl ?
The 2.7 user storeurl for everything and I mean everything till the last
column.
So what do you think?

It seems to me like giving it a public name of storeurl is the more
convenient way for public naming.(config etc)

>
>
> Thank you,
>
> Alex.
>
NP
Eliezer
Received on Mon Sep 10 2012 - 16:17:01 MDT