Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various changes that I have acumulated over time #42

Closed
wants to merge 13 commits into from

Conversation

verpeteren
Copy link
Contributor

  1. I have added the documentation to the source so that it is easier to maintain and run and update the website-documentation.
  2. I grouped moved the getHostByName and the readfile functions out the Ape namespace and put them into the Ape.os namespace, also added a resolveHostByName that uses udns
  3. I added a os.system, os.writefile, eval function
  4. fixed some typo's, cleared some whitespace, added some todo, fixed another problem in the build.sh
  5. fixed a bug with a missing fallthrough
  6. build.sh, Makefile and module/Makefile work very nice
  7. fixed a segfault when when the configfile could not be found.
  8. improved and uniform logging upon startup of the daemon
  9. main.ape.js stays the default entry point, but an alternative can be entered via the modules/conf/javacript.conf with a autoexec parameter

@ptejada
Copy link
Contributor

ptejada commented Sep 14, 2013

Nice! I'll be testing this patch in my environment. The os namespace is a good idea

@ptejada
Copy link
Contributor

ptejada commented Apr 20, 2014

What is the status on this pull request?

@lcharette
Copy link
Collaborator

I don't think I ever tested this. I probably should. Have you Pablo?

Again, I didn't test, but are the os, eval and hostname functions asynchronous and non-blocking?

@ptejada
Copy link
Contributor

ptejada commented Apr 20, 2014

I did not tested the new features of reading and writing files but I did
loaded the patch onto my server a while back and did not experienced any
errors or slow downs to the overall server.

On Sun, Apr 20, 2014 at 5:19 PM, Louis Charette notifications@github.comwrote:

I don't think I ever tested this. I probably should. Have you Pablo?

Again, I didn't test, but are the os, eval and hostname functions
asynchronous and non-blocking?


Reply to this email directly or view it on GitHubhttps://github.com//pull/42#issuecomment-40905188
.

@lcharette
Copy link
Collaborator

I did some quick testing. Overall, everything looks good. But I found small issues:

os.system doesn't work asynchronously. You can test this in server side test /scripts/test/Systemcmd.js. If you replace the command with something that take a long time, for example a large download...

var r = os.system('/usr/bin/wget', 'http://releases.ubuntu.com/14.04/ubuntu-14.04-desktop-amd64.iso -P /tmp/');

...the next test won't execute until the download is not finished. The clients won't respond also. I know it would be stupid to run this king of long task with Ape, but if a small normal task was to hang for some reason, APE wouldn't push anything at all. Also, I didn't test if it works, but it could be dangerous if command could be executed as root.

As for resolveHostByName, the third test in /scripts/test/Hostname.js which isn't supposed to work, I don't get a callback at all. I fallowed what @verpeteren wrote in the source code:

var r = os.resolveHostByName('www.this.should.not.work.@all', function(ip) {
        if (! ip) {
            Ape.log('os.resolveHostByName (3): Could not resolve host' );
        } else {
            Ape.log('os.resolveHostByName (3): ' + ip);
        }
    });

But I don't get any callback ("Could not resolve host"). Or is the timeout is too long?

Finally, in the test file /scripts/test/FileReadWrite.js, this line doesn't work: r = os.writefile(fn, rot13, true);. I get:

./aped: symbol lookup error: ../modules/lib/libmod_spidermonkey.so: undefined symbol: S_ISREG

I'm no C expert, but from what I read, #include <sys/stat.h> needs to be added to modules/libape-spidermonkey.c

@lcharette
Copy link
Collaborator

OK, I found a fix for resolveHostByName. I'll merge this and commit the fix for resolveHostByName and FileReadWrite afterward.

@lcharette
Copy link
Collaborator

This as been merge in commit : 9165f30

@lcharette lcharette closed this Apr 25, 2014
lcharette added a commit that referenced this pull request Apr 25, 2014
lcharette added a commit that referenced this pull request Apr 25, 2014
@lcharette
Copy link
Collaborator

See last 3 commits for the fix I talked about earlier. I also improve the test so it's easier to read in the console. https://github.com/APE-Project/APE_Server/commits/master

If everything is fine with the current state of the branch, maybe we could release it as 1.1.3 and start 1.1.4-DEV ?

@lcharette
Copy link
Collaborator

@verpeteren, did you write the documentation for the website regarding os.writefile and the new function included in this?

@verpeteren
Copy link
Contributor Author

Hai

yes, around line 2970 in modules/libape-spidermonkey.c
you might want to rebuild the documentation. here is the tool that i
made https://github.com/verpeteren/Docstrape/tree/master/example

Peter

Louis Charette schreef op 2014-05-14 18:28:

@verpeteren [1], did you write the documentation for the website
regarding os.writefile and the new function included in this?

Reply to this email directly or view it on GitHub [2].

Links:

[1] https://github.com/verpeteren
[2]
#42 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants