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

Hanging on "Discovering packages" #191

Open
mtulla opened this issue Dec 3, 2024 · 12 comments
Open

Hanging on "Discovering packages" #191

mtulla opened this issue Dec 3, 2024 · 12 comments

Comments

@mtulla
Copy link

mtulla commented Dec 3, 2024

This extension seems super useful, and I'm trying to get it adopted by my lab for our AI planning workflows. However, I'm running into some issues when I try to use the default solver.planning.domains or a self-hosted server running planning-as-a-service as my planning engine.

If I try to use the default planning engine configuration with solver.planning.domains, it hangs on "Discovering packages". The screenshot below shows what I mean.
Screenshot 2024-12-03 at 3 55 02 PM

I thought this was due to a problem with solver.planning.domains. Regardless, I wanted to self-host a planning-as-a-service server, so I went ahead and did that following the instructions on the GitHub. The server seems to be working correctly, and when I run the python script mentioned in the project and copied below, I receive a correct plan.

import time
from pprint import pprint

import requests

req_body = {
    "domain": "(define (domain BLOCKS) (:requirements :strips) (:predicates (on ?x ?y) (ontable ?x) (clear ?x) (handempty) (holding ?x) ) (:action pick-up :parameters (?x) :precondition (and (clear ?x) (ontable ?x) (handempty)) :effect (and (not (ontable ?x)) (not (clear ?x)) (not (handempty)) (holding ?x))) (:action put-down :parameters (?x) :precondition (holding ?x) :effect (and (not (holding ?x)) (clear ?x) (handempty) (ontable ?x))) (:action stack :parameters (?x ?y) :precondition (and (holding ?x) (clear ?y)) :effect (and (not (holding ?x)) (not (clear ?y)) (clear ?x) (handempty) (on ?x ?y))) (:action unstack :parameters (?x ?y) :precondition (and (on ?x ?y) (clear ?x) (handempty)) :effect (and (holding ?x) (clear ?y) (not (clear ?x)) (not (handempty)) (not (on ?x ?y)))))",
    "problem": "(define (problem BLOCKS-4-0) (:domain BLOCKS) (:objects D B A C ) (:INIT (CLEAR C) (CLEAR A) (CLEAR B) (CLEAR D) (ONTABLE C) (ONTABLE A) (ONTABLE B) (ONTABLE D) (HANDEMPTY)) (:goal (AND (ON D C) (ON C B) (ON B A))) )",
}

# Send job request to solve endpoint
solve_request_url = requests.post(
    "http://<my_server_ip>:5001/package/lama-first/solve", json=req_body
).json()

# Query the result in the job
celery_result = requests.post(
    "http://<my_server_ip>:5001" + solve_request_url["result"]
)

print("Computing...")
while celery_result.json().get("status", "") == "PENDING":

    # Query the result every 0.5 seconds while the job is executing
    celery_result = requests.post(
        "http://<my_server_ip>:5001" + solve_request_url["result"]
    )
    time.sleep(0.5)

pprint(celery_result.json())

However, when I try to point the vscode extension to my server, I get the exact same hanging on "Discovering packages". The URL I am using is https://your-planning-as-a-service:5001/package, as the extension suggests.

Any ideas on how to fix this?

@haz
Copy link

haz commented Dec 3, 2024

Can you paste a screenshot of your configuration?

@mtulla
Copy link
Author

mtulla commented Dec 3, 2024

Screenshot 2024-12-03 at 4 16 46 PM Screenshot 2024-12-03 at 4 21 57 PM

@haz
Copy link

haz commented Dec 3, 2024

Thanks -- I think we may be facing some certificate issues. I'll have a look in a few hours.

@haz
Copy link

haz commented Dec 4, 2024

Want to try again? We had an expired SSL cert, and renewing seems to have fixed some other behaviour.

@mtulla
Copy link
Author

mtulla commented Dec 4, 2024

@haz solver.planning.domains works now! I also fixed the issue with my server setup by using HTTP instead of HTTPS. I changed the URL to http://<my_server_ip>:5001/package instead of https://<my_server_ip>:5001/package in the configuration and everything started working.

Thanks for the help! I have two suggestions as well.

  • The extension shouldn't hang on "Discovering packages" if it can't get a response after a given timeout or if it gets a bad response. If it doesn't get a response—due to certificate issues or any other problems—a helpful error message should be shown.

  • I think the default placeholder URL when a user is creating a new planning-as-a-service configuration should use HTTP instead of HTTPS to avoid this issue. As the screenshot below shows, currently the user is prompted to change the placeholder URL to their server's address when setting up a new PaaS configuration. Wouldn't it be better if that placeholder URL was http://your-planning-as-a-service:5001/package instead of HTTPS? This makes it much harder to run into this kind of issue. If a user wants to use SSL they can change it to HTTPS, but at least they'll be much less likely to run into this kind of certificate issue.

I can try to submit a PR with these changes if you want.

Screenshot 2024-12-04 at 12 15 34 AM

@haz
Copy link

haz commented Dec 6, 2024

Thanks, @mtulla , a PR would be great! Perhaps 2 separate ones...the first point is an obvious one that could use a fix, but the second is a design decision -- a common usecase for this is to have a company deploy their own solver endpoint in-house, and I'm not sure if the common practice is to put SSL authentication on, or just leave it HTTP.

Any thoughts, @jan-dolejsi ?

@mtulla
Copy link
Author

mtulla commented Dec 10, 2024

Hey @haz. I tried to set up my development environment to work on those PRs. When I run npm run compile I get the following error though:

❯ npm run compile

> [email protected] compile
> tsc -p ./ && copyfiles --flat ./src/planView/model/package.json ./out/planView/model/ && copyfiles --flat ./src/modelView/model/package.json ./out/modelView/model/ && cd views/common && npm run compile && cd ../searchview && npm run compile && cd ../planview && npm run compile && cd ../modelView && npm run compile && cd ../..


> [email protected] compile
> copyfiles --flat ../../node_modules/pddl-gantt/styles/* ./


> [email protected] compile
> npm run lint && browserify src/search.ts -p tsify > out/search.js


> [email protected] lint
> eslint src --ext ts

(node:60403) [DEP0060] DeprecationWarning: The `util._extend` API is deprecated. Please use Object.assign() instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

> [email protected] compile
> npm run lint && browserify src/plans.ts -p tsify > out/plans.js


> [email protected] lint
> eslint src --ext ts

(node:60529) [DEP0060] DeprecationWarning: The `util._extend` API is deprecated. Please use Object.assign() instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
TypeScript error: /users/miguel/projects/vscode-pddl/views/planview/test/test.ts(1,24): Error TS1261: Already included file name '/users/miguel/projects/vscode-pddl/node_modules/@types/chai/index.d.ts' differs from file name '/Users/Miguel/Projects/vscode-pddl/node_modules/@types/chai/index.d.ts' only in casing.
  The file is in the program because:
    Imported via 'chai' from file '/users/miguel/projects/vscode-pddl/views/planview/test/test.ts' with packageId '@types/chai/[email protected]'
    Entry point for implicit type library 'chai' with packageId '@types/chai/[email protected]'
    Type library referenced via 'chai' from file '/Users/Miguel/Projects/vscode-pddl/node_modules/@types/chai-string/index.d.ts' with packageId '@types/chai/[email protected]'
    Entry point for implicit type library 'chai' with packageId '@types/chai/[email protected]'

I'm running on an M1 Macbook. From what I read online, I thought this line in tsconfig.json (not something I added) is supposed to fix this.

		"forceConsistentCasingInFileNames": true,

Do you have any idea on how to fix this? I don't really have experience working with VSCode Extensions.

@haz
Copy link

haz commented Dec 10, 2024

I'm afraid this one is way beyond my expertise :( . I handle the server side of things, and it would need to be @jan-dolejsi that chimes in here.

@mtulla
Copy link
Author

mtulla commented Dec 10, 2024

No worries! I'll wait to see what @jan-dolejsi says.

@jan-dolejsi
Copy link
Owner

Thanks, @mtulla , a PR would be great! Perhaps 2 separate ones...the first point is an obvious one that could use a fix, but the second is a design decision -- a common usecase for this is to have a company deploy their own solver endpoint in-house, and I'm not sure if the common practice is to put SSL authentication on, or just leave it HTTP.

Any thoughts, @jan-dolejsi ?

I applied the secure by default principle. If the service is hosted at a specific URL, I would expect the user to past the URL. Including the http/s.
If the service accepts both http and https traffic, we should default to https as that is protecting the payload from attacks.

@jan-dolejsi
Copy link
Owner

Hey @haz. I tried to set up my development environment to work on those PRs. When I run npm run compile I get the following error though:

❯ npm run compile

> [email protected] compile
> tsc -p ./ && copyfiles --flat ./src/planView/model/package.json ./out/planView/model/ && copyfiles --flat ./src/modelView/model/package.json ./out/modelView/model/ && cd views/common && npm run compile && cd ../searchview && npm run compile && cd ../planview && npm run compile && cd ../modelView && npm run compile && cd ../..


> [email protected] compile
> copyfiles --flat ../../node_modules/pddl-gantt/styles/* ./


> [email protected] compile
> npm run lint && browserify src/search.ts -p tsify > out/search.js


> [email protected] lint
> eslint src --ext ts

(node:60403) [DEP0060] DeprecationWarning: The `util._extend` API is deprecated. Please use Object.assign() instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

> [email protected] compile
> npm run lint && browserify src/plans.ts -p tsify > out/plans.js


> [email protected] lint
> eslint src --ext ts

(node:60529) [DEP0060] DeprecationWarning: The `util._extend` API is deprecated. Please use Object.assign() instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
TypeScript error: /users/miguel/projects/vscode-pddl/views/planview/test/test.ts(1,24): Error TS1261: Already included file name '/users/miguel/projects/vscode-pddl/node_modules/@types/chai/index.d.ts' differs from file name '/Users/Miguel/Projects/vscode-pddl/node_modules/@types/chai/index.d.ts' only in casing.
  The file is in the program because:
    Imported via 'chai' from file '/users/miguel/projects/vscode-pddl/views/planview/test/test.ts' with packageId '@types/chai/[email protected]'
    Entry point for implicit type library 'chai' with packageId '@types/chai/[email protected]'
    Type library referenced via 'chai' from file '/Users/Miguel/Projects/vscode-pddl/node_modules/@types/chai-string/index.d.ts' with packageId '@types/chai/[email protected]'
    Entry point for implicit type library 'chai' with packageId '@types/chai/[email protected]'

I'm running on an M1 Macbook. From what I read online, I thought this line in tsconfig.json (not something I added) is supposed to fix this.

		"forceConsistentCasingInFileNames": true,

Do you have any idea on how to fix this? I don't really have experience working with VSCode Extensions.

Now that is a weird one. The difference in casing is in the /Users/Miguel/Project, which is outside of the control of the workspace and scripts within. Not sure which part of the chain is converting the path the all-lowercase. That itself would deserve some googling time.
But let's try to address the symptom here first. Flip the "forceConsistentCasingInFileNames": true, to false in the tsconfig.json in the repo root directory as you suggested.
The forceConsistentCasingInFileNames is there just to make sure I do not commit files in mismatching casing (which happens when you refactor code and change casing of file names), because I develop on Windows and everything transpiles and runs fine there. So to prevent a build pipeline failure, the forceConsistentCasingInFileNames helps me fail early. But that is not your case, so just remove the line or flip it to false and try again.

@jan-dolejsi
Copy link
Owner

No, the exception should not hang while discovering packages. There are still some unexpected failure points that the code is not handing by raising an error message.

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

No branches or pull requests

3 participants