Forum OpenACS Development: Re: approach to refactoring ACS Mail Lite

Collapse
Posted by Benjamin Brink on
errata

"acs_mail_lite::get_parameter vs. acs_mail_lite::get_parameter"

becomes

"acs_mail_lite::get_parameter vs. parameter::get or parameter::get_from_package_key"

turbulence happens

Collapse
Posted by Iuri Sampaio on
Hi Ben,

Interesting post. I was going to play with pad_procs mentioned, but unfortunately, on my installation they don't work.

I've created a new parameter, of a new custom pkg, then called it as in the example bellow. ci_url is void/null. I could have added the default switch, but that'd masquerade the actual issue. I'm about to got deeper debugging ad_procs ::get and get_from_package_key...

set ci_url [parameter::get \
-parameter CodeIgniterWebsiteUrl \
-package_id [ad_conn package_id]]

Best wishes,

Collapse
Posted by Benjamin Brink on
Hi Iuri,

Maybe parameter values don't update until nsd restart for the same reason that scheduled procs don't update on package reload via acs-admin/apm.

Making these updates consistent with other package reload features appears on target for OpenACS 5.10 release in the agenda/wish-list item: "dynamic reloading reform, including support for scheduled procedures": http://openacs.org/xowiki/openacs-todo

Your findings and contributions are welcome.

best wishes,
Ben

Collapse
Posted by Iuri Sampaio on
It's something else. I had restarted Naviserver. A couple of time actually while I was debugging, but I still haven`t found the cause.

Best wishes,

Collapse
Posted by Benjamin Brink on
Hi Iuri,

If you can find a way to replicate the issue, then maybe someone can help identify the cause.

Are you using that code snipet in a scheduled proc?

If so, [ad_conn package_id] would normally throw an error, because there is no connection to get a package_id.

However, a catch statement somewhere in parameter::get is filtering the error, and returning empty string instead. Is this what you are seeing?

cheers,
Ben

Collapse
Posted by Benjamin Brink on
Actually, ad_conn now catches the error. Here is example code in a package-init.tcl file, and how error is reported as a "Notice" in log:

set package_id [ad_conn package_id]
ns_log Notice "ad_conn package_id = '${package_id}'"

Notice: request processor did not set <ad_conn package_id>, fallback: 624
[25/Jul/2017:19:18:09][1826.7f0d32aca740][-main-] Notice: ad_conn package_id = '624'

Subsequently, your code example tries to get value from a different package.

Collapse
Posted by Iuri Sampaio on
Ben,

Neither assigning a static value for package_id, nor creating a whole new package, with a new parameter worked fine. I ran the tests in the acs-developer shell

Example: parameter::get_from_package_key -package_key "evex-users" -parameter "ExternalWebsiteURL"

Both ad_procs [parameter::get] or [parameter::get_from_package_key] return blank (i.e. "") as value.

p.s. Parameter's accessible through APM -> (evex-users) pkg admin -> parameters

Best wishes,
Iuri

Collapse
Posted by Benjamin Brink on
Hi Iuri,

Sounds like a gremlin in the system!

Surely it's time to fill those procs with ns_log Debug comments.. and set Dev or Debug to on in /admin/nsstats.tcl

Maybe issue is temporal, taking a few minutes after restart to complete background processes?

Anyway, certainly worth investigating.
best wishes,
Ben

Collapse
Posted by Gustaf Neumann on
There is nothing wrong with the ad_procs ::get and get_from_package_key, there are used all over the system. If these functions would be broken, we would have big troubles. When someone adds a new parameter to an existing package, it is not sufficient to update the .info file via the package manager, but one has to increment the version number of the package to be abble to run an upgrade; In the upgrade step, the parameters are actually added to the database. Providing a non-default value to this new parameter can be done after running the upgrade generically via the web-interface in /admin/site-map.

Note that parameter::get_from_package_key is supposed to work only for singleton packages. New packages should use global parameters instead (see [1]).

Hope this helps
-gn

[1] http://openacs.org/api-doc/proc-view?proc=parameter::get_from_package_key&source_p=1

Collapse
Posted by Iuri Sampaio on
Thanks Gustaf,

I've done all that. I believe the problem exists because, instead of writing it directly within the file .info, I'd created the parameter through the interface (APM). Afterwards, I confirmed that the parameter was automatically generate within .info, then increased manually the version number and restarted NS, but unfortunately ad_procs still persist returning "".

My package isn't singleton, but I did use global scope.

parameter scope="global" datatype="string" min_n_values="1" max_n_values="1" name="CodeIgniterWebsiteUrl" default="http://dev.evex.co" description="Website URL for Venues & Services CMS in CodeIgniter"/

I'll debug this problem over the weekend and let you .

Getting back to the subject of refactoring ACS Mail Lite, I can help to do the job. Let me know if you guys need want my help.

Best wishes,

Collapse
Posted by Gustaf Neumann on
As mentioned above, you have to run an "upgrade from the local file system" to process the changes in the .info file. just restarting the server after updating the info file is not enough.

In order to access the global parameter, one has to use parameter::get_global_value [1] instead of parameter::get. it might be an option to work towards unifying these calls, there is an agenda-point for parameters in the the 5.10.0 agenda.

-gn
[1] http://openacs.org/api-doc/proc-view?proc=parameter%3A%3Aget_global_value

Collapse
Posted by Iuri Sampaio on
Thanks Gustaf,
ad_proc parameter::get_global_value worked well.
It'd be great if I could participate in 5.10.0 agenda.
Best wishes,