python-domino icon indicating copy to clipboard operation
python-domino copied to clipboard

Updates for ITG requests

Open vaibhav-dhawan opened this issue 4 years ago • 11 comments

  • Ability to initialise without a project defined - specifically requested by ITG admins. This does leave some endpoints unusable in routes, have added warning and exceptions when trying to use these routes without specifying a project.
  • Orgs_list, users_list calls for admins - along the lines of environments_list, models_list. Orgs list only works for sysadmins

Jira: https://dominodatalab.atlassian.net/browse/DOM-33605

vaibhav-dhawan avatar Oct 25 '21 15:10 vaibhav-dhawan

  • Ability to initialise without a project defined - specifically requested by ITG admins. This does leave some endpoints unusable in routes, though i can't find a good way around that at the moment.

A public API change that leaves some endpoints in an unusable state does not seem like a shippable solution to me.

Is there a Jira ticket that explains these ITG requests?

dknupp avatar Oct 25 '21 18:10 dknupp

  • Ability to initialise without a project defined - specifically requested by ITG admins. This does leave some endpoints unusable in routes, though i can't find a good way around that at the moment.

A public API change that leaves some endpoints in an unusable state does not seem like a shippable solution to me.

Is there a Jira ticket that explains these ITG requests?

@dknupp : actually i was thinking about a default project for this one, but in looking over the ITG response I remember now that they're creating admins without the practitioner role. That means those admins won't even have a default project associated. That's why they're having to create a dummy project for the admin, and then put that name into the code (see https://dominodatalab.atlassian.net/browse/DOM-33605 for their comments).

So, my thought was to still allow for the null project name, but print a warning when initialising that the runs, files/commits/blobs and model functions won't work unless initialised with a project. I can also add a bit of code to the routes helper functions to throw an exception when trying to use a null project name

vaibhav-dhawan avatar Oct 26 '21 11:10 vaibhav-dhawan

@vaibhav-dhawan -- Will get back to this review ASAP. A few others fires burning at the moment need attention.

dknupp avatar Oct 28 '21 18:10 dknupp

@vaibhav-dhawan -- Will get back to this review ASAP. A few others fires burning at the moment need attention.

No problem, whenever you can! thanks

vaibhav-dhawan avatar Oct 29 '21 10:10 vaibhav-dhawan

@dknupp please take a look when you have time

ddl-alexpanin avatar Nov 15 '21 16:11 ddl-alexpanin

@dknupp please take a look when you have time

This change causes several test failures. I need to find time to dig into this.

% DOMINO_API_HOST=<redacted> DOMINO_USER_NAME=<redacted> DOMINO_USER_API_KEY=<redacted> pytest -sv tests/test_basic_auth.py tests/test_environments.py tests/test_jobs.py tests/test_models.py tests/test_projects.py
===================================== test session starts =====================================
platform darwin -- Python 3.9.7, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /Users/davidknupp/Documents/code/dominodatalab/python-domino/foo/bin/python
cachedir: .pytest_cache
rootdir: /Users/davidknupp/Documents/code/dominodatalab/python-domino, configfile: pytest.ini
plugins: requests-mock-1.9.3
collected 32 items

tests/test_basic_auth.py::test_no_auth_type_error PASSED
tests/test_basic_auth.py::test_invalid_token_file_error PASSED
tests/test_basic_auth.py::test_malformed_project_input_error PASSED
tests/test_basic_auth.py::test_object_creation_with_auth_token PASSED
tests/test_basic_auth.py::test_object_creation_with_api_key PASSED
tests/test_basic_auth.py::test_object_creation_with_token_file PASSED
tests/test_basic_auth.py::test_re_authentication_with_a_new_type PASSED
tests/test_basic_auth.py::test_auth_token_precedence PASSED
tests/test_basic_auth.py::test_token_file_precedence PASSED
tests/test_basic_auth.py::test_auth_against_real_deployment_with_api_key PASSED
tests/test_basic_auth.py::test_auth_against_real_deployment_with_token_file SKIPPED (No token file in environment)
tests/test_environments.py::test_list_envirtonments PASSED
tests/test_environments.py::test_list_useable_envirtonments FAILED
tests/test_jobs.py::test_job_status_completes_with_default_params FAILED
tests/test_jobs.py::test_job_status_ignores_RequestException_and_times_out FAILED
tests/test_jobs.py::test_job_status_without_ignoring_exceptions FAILED
tests/test_jobs.py::test_job_start_blocking FAILED
tests/test_jobs.py::test_runs_list FAILED
tests/test_jobs.py::test_queue_job FAILED
tests/test_models.py::test_list_models FAILED
tests/test_models.py::test_publish_a_model FAILED
tests/test_models.py::test_publish_model_version FAILED
tests/test_projects.py::test_project_create PASSED
tests/test_projects.py::test_project_fork FAILED
tests/test_projects.py::test_list_commits FAILED
tests/test_projects.py::test_list_files_in_commit FAILED
tests/test_projects.py::test_upload_file_to_project FAILED
tests/test_projects.py::test_get_file_from_a_project FAILED
tests/test_projects.py::test_add_and_remove_project_collaborator FAILED
tests/test_projects.py::test_publish_app_from_a_project FAILED
tests/test_projects.py::test_unpublish_app_from_a_project FAILED
tests/test_projects.py::test_archiving_non_existent_project_raises_appropriate_error PASSED

dknupp avatar Nov 15 '21 18:11 dknupp

Hmm ok I didn't even see the tests - I can look into the failures @dknupp and see what's going on

vaibhav-dhawan avatar Nov 16 '21 11:11 vaibhav-dhawan

@dknupp ok with my amazing git skills i've managed to FUBAR this branch, somehow. I was just trying to rebase on master so i can run the full set of tests, but now it's showing a bunch of extra changes in my PR. I'll close this and submit a fresh one, but i figured out the issues with testing:

  • Problem with my check_project function which is resolved
  • In my testing (inside a domino workspace), the full tests failed because of the 'jobs' tests - these were running for a few minutes, and in that time the token appears to have expired. After that the models/environments tests fail with an auth error. I was able to run the 2 sets of tests separately and get them all to pass: DOMINO_USER_NAME=integration-test DOMINO_API_HOST=https://vsd-aks-119.cs.domino.tech pytest -sv tests/test_models.py tests/test_projects.py DOMINO_USER_NAME=integration-test DOMINO_API_HOST=https://vsd-aks-119.cs.domino.tech pytest -sv tests/test_basic_auth.py tests/test_environments.py tests/test_jobs.py

But, will redo in a new branch and open a new PR

vaibhav-dhawan avatar Nov 16 '21 13:11 vaibhav-dhawan

Nevermind! Google to the rescue: https://stackoverflow.com/questions/16306012/github-pull-request-showing-commits-that-are-already-in-target-branch. Ignoring the warning here, since the dependencies were already installed anyway. Test results:

(base) ubuntu@run-6193a37676c9fc5205751383-r9xqk:/repos/python-domino$ DOMINO_USER_NAME=integration-test DOMINO_API_HOST=https://vsd-aks-119.cs.domino.tech pytest -sv tests/test_models.py tests/test_projects.py --log-cli-level=INFO
========================================================================= test session starts =========================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /opt/conda/bin/python
cachedir: .pytest_cache
rootdir: /repos/python-domino, configfile: pytest.ini
plugins: requests-mock-1.9.3, anyio-3.3.0
collected 13 items                                                                                                                                                    

tests/test_models.py::test_list_models 
--------------------------------------------------------------------------- live log setup ----------------------------------------------------------------------------
INFO     domino.domino:domino.py:55 Domino deployment https://vsd-aks-119.cs.domino.tech is running version 4.6.2
PASSED
tests/test_models.py::test_publish_a_model PASSED
tests/test_models.py::test_publish_model_version PASSED
tests/test_projects.py::test_project_create 
--------------------------------------------------------------------------- live log setup ----------------------------------------------------------------------------
INFO     domino.domino:domino.py:55 Domino deployment https://vsd-aks-119.cs.domino.tech is running version 4.6.2
PASSED
tests/test_projects.py::test_project_fork PASSED
tests/test_projects.py::test_list_commits PASSED
tests/test_projects.py::test_list_files_in_commit PASSED
tests/test_projects.py::test_upload_file_to_project PASSED
tests/test_projects.py::test_get_file_from_a_project PASSED
tests/test_projects.py::test_add_and_remove_project_collaborator PASSED
tests/test_projects.py::test_publish_app_from_a_project PASSED
tests/test_projects.py::test_unpublish_app_from_a_project PASSED
tests/test_projects.py::test_archiving_non_existent_project_raises_appropriate_error 
---------------------------------------------------------------------------- live log call ----------------------------------------------------------------------------
INFO     domino.domino:domino.py:55 Domino deployment http://domino.somefakecompany.com is running version 9.9.9
PASSED

========================================================================== warnings summary ===========================================================================
../../home/ubuntu/.local/lib/python3.8/site-packages/_pytest/config/__init__.py:1233
  /home/ubuntu/.local/lib/python3.8/site-packages/_pytest/config/__init__.py:1233: PytestConfigWarning: Unknown config option: env_files
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=================================================================== 13 passed, 1 warning in 12.19s ====================================================================
(base) ubuntu@run-6193a37676c9fc5205751383-r9xqk:/repos/python-domino$ DOMINO_USER_NAME=integration-test DOMINO_API_HOST=https://vsd-aks-119.cs.domino.tech pytest -sv tests/test_basic_auth.py tests/test_environments.py tests/test_jobs.py 
========================================================================= test session starts =========================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /opt/conda/bin/python
cachedir: .pytest_cache
rootdir: /repos/python-domino, configfile: pytest.ini
plugins: requests-mock-1.9.3, anyio-3.3.0
collected 19 items                                                                                                                                                    

tests/test_basic_auth.py::test_no_auth_type_error PASSED
tests/test_basic_auth.py::test_invalid_token_file_error PASSED
tests/test_basic_auth.py::test_malformed_project_input_error PASSED
tests/test_basic_auth.py::test_object_creation_with_auth_token PASSED
tests/test_basic_auth.py::test_object_creation_with_api_key PASSED
tests/test_basic_auth.py::test_object_creation_with_token_file PASSED
tests/test_basic_auth.py::test_re_authentication_with_a_new_type PASSED
tests/test_basic_auth.py::test_auth_token_precedence PASSED
tests/test_basic_auth.py::test_token_file_precedence PASSED
tests/test_basic_auth.py::test_auth_against_real_deployment_with_api_key PASSED
tests/test_basic_auth.py::test_auth_against_real_deployment_with_token_file PASSED
tests/test_environments.py::test_list_envirtonments PASSED
tests/test_environments.py::test_list_useable_envirtonments PASSED
tests/test_jobs.py::test_job_status_completes_with_default_params PASSED
tests/test_jobs.py::test_job_status_ignores_RequestException_and_times_out PASSED
tests/test_jobs.py::test_job_status_without_ignoring_exceptions PASSED
tests/test_jobs.py::test_job_start_blocking PASSED
tests/test_jobs.py::test_runs_list PASSED
tests/test_jobs.py::test_queue_job Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 has not completed.
Job 6193b6c976c9fc52057513b5 succeeded.
PASSED

========================================================================== warnings summary ===========================================================================
../../home/ubuntu/.local/lib/python3.8/site-packages/_pytest/config/__init__.py:1233
  /home/ubuntu/.local/lib/python3.8/site-packages/_pytest/config/__init__.py:1233: PytestConfigWarning: Unknown config option: env_files
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

-- Docs: https://docs.pytest.org/en/stable/warnings.html
============================================================== 19 passed, 1 warning in 233.22s (0:03:53) ==============================================================

vaibhav-dhawan avatar Nov 16 '21 14:11 vaibhav-dhawan

  • full tests failed because of the 'jobs' tests - these were running for a few minutes, and in that time the token appears to have expired.

This is actually weird, token from file is now re-read before each request: https://github.com/dominodatalab/python-domino/pull/105

ddl-alexpanin avatar Nov 16 '21 17:11 ddl-alexpanin

@ddl-alexpanin just tried again, it worked this time. But it may well be a timing thing, the jobs were probably taking longer last time. I was definitely seeing auth failures on everything after the jobs tests and there was no other reason for it (running my own tests on the same modules worked fine)

vaibhav-dhawan avatar Nov 17 '21 13:11 vaibhav-dhawan

Way too out of date, closing now

vaibhav-dhawan avatar Apr 13 '23 10:04 vaibhav-dhawan