search

FNNDSC / pman

A process management system written in python
MIT License
23 stars 33 forks source link

Fix cmdParse and mostly refactor internal behaviour for internal memory clearing #84

Closed rudolphpienaar closed 6 years ago

rudolphpienaar commented 6 years ago

Most of this PR deals with sending pman an instruction to clear its internal memory (the memory analog of the /tmp/pman DB). This is useful for cases when a client, typically in a test/retest scenario, wants to send the same test job ID again and again. In such a case, the internal DB must be cleared.

rudolphpienaar commented 6 years ago

Hey @danmcp and @ravisantoshgudimetla --

So some fixes to pman that address the cmdParse and also some refactoring to address some extra behavior -- basically handling the ability to clear the internal in-memory DB (and also /tmp/pman).

Also added machinery to start pman dock from new docker-compose etc.

LMKWYT

danmcp commented 6 years ago

@rudolphpienaar Can you please squash these commits.

rudolphpienaar commented 6 years ago

Sure -- I usually do the squash when I merge. If you are ok with that, I'll go ahead then

danmcp commented 6 years ago

@rudolphpienaar You have a lot of files that have permissions changed. What's the reasoning there?

What's the use case for clearing the internal memory exactly?

rudolphpienaar commented 6 years ago

The case is mostly test/retest. Right now if you send jobs to pman you have to make certain that each job has a unique job-id. This can make batch testing the same battery of tests problematic.

By clearing the internal memory, we can do retests in a loop.

At the same time, while doing this, I discovered some design / logic bugs that I fixed. These fixes are actually more critical than the clear internal memory.

rudolphpienaar commented 6 years ago

The permission change was an accidental (but not relevant) artifact of reusing the CUBE make script and docker-compose here.

rudolphpienaar commented 6 years ago

Hi @ravisantoshgudimetla -- see in the code the "and False" with the cmdParse checking. Right now it means that no cmdParsing is done.

Also, @danmcp -- I'd like to squash and merge if you are ok with it? I know the whole "ptree" component will soon be deprecated, but this PR fixes an error that does exist in the design.

e2e tests all pass.

danmcp commented 6 years ago

@rudolphpienaar I don't have any issue with a temp fix for using ptree in tests. I didn't have any other concerns other than the one @ravisantoshgudimetla had about whether more logic from cmdParse should be removed as part of this pull.