-
Notifications
You must be signed in to change notification settings - Fork 0
Common Client and separate API objects #28
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
Conversation
ojkoenig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked out your branch and played with an example. I am happy with the changes.
I understand the needed backend change to move selections endpoint to projects/my_proj/job_selections and can help on that if needed.
But what backend change you need on algorithm?
Does the merge of this PR depend on a possible keycloak-python change you mention?
@ojkoenig Thanks for reviewing. No backend changes needed for algorithms - the test for algorithms hits the selections endpoint as well and therefore it's currently failing, I only need to make sure it passes once the backend changes for selections are in. With respect to KeyCloak: currently the Auth API (get\create\update\delete users) is disabled because it would require storing the password inside our |
| project_api = ProjectApi(client, proj.id) | ||
|
|
||
| cwd = os.path.dirname(__file__) | ||
| example_dir = os.path.join(cwd, "..", "..", "examples", "mapdl_motorbike_frame") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd upload any examples that cannot or should not be included in repository to https://github.com/pyansys/example-data and implement a basic downloader as in https://github.com/pyansys/pymapdl/blob/main/src/ansys/mapdl/core/examples/downloads.py or using pooch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, I agree we should do that eventually. Jonathan did the same already for pyAssembly.
@FedericoNegri : Sounds like a good plan to me. |
Example of new syntax:
All tests and example are updated. There are some open todo items (related to this PR):
KeycloakAdminwith existing tokens, to avoid storing the password. Might need a PR to https://github.com/marcospereirampj/python-keycloak --> agreed to postpone to a follow-up PRThe 2 current test failures are due to the Auth issue above
