Skip to content

Conversation

@liudmylaru
Copy link
Member

@liudmylaru liudmylaru commented Sep 20, 2023

Description

After we setup integration tests to run with instance principals we got 42 tests failed. This PR is to address integration tests and opctl tests. More fixes will follow in future PRs.
Jira for ALL Integration tests: https://jira.oci.oraclecorp.com/browse/ODSC-47592 (details in comments in subtasks)
Jira for OPCTL tests: https://jira.oci.oraclecorp.com/browse/ODSC-47603

In addition to code changed in the tests in this PR, there were updated done to Dynamic Groups and Policy so that instance principal will work with logs, buckets and dataflow service.

What was done

  • removed hardcoded config path from "jobs" folder
  • added if to update kwargs with default_signer in jobs serializer write_to_fle
  • updated tests in "other" folder - mainly harcdoded ~/.oci/config
  • fixed check-docstring-first pre-commit check in serializer.py
  • updated opctl integration tests and fixed issues

Pre-commit

check python ast.........................................................Passed
check docstring is first.............................(no files to check)Skipped
check json...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check yaml...........................................(no files to check)Skipped
detect private key.......................................................Passed
fix end of files.........................................................Passed
pretty format json...................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
black....................................................................Passed
rst ``code`` is two backticks........................(no files to check)Skipped
rst ``inline code`` next to normal text..............(no files to check)Skipped
Detect hardcoded secrets.................................................Passed
check-copyright..........................................................Passed
[ODSC-47592/fix_int_test_jobs_python_runtime 5455bfb] ODSC-47592. Fix test_jobs_python_runtime.py

Integration tests

Integration tests (no opctl included) passed with Python3.9:
image

Integration OPCTL tests passed on Python3.8:
image

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 20, 2023
@liudmylaru liudmylaru changed the title ODSC-47592. Fix test_jobs_python_runtime.py Fix test_jobs_python_runtime.py Sep 20, 2023
@github-actions
Copy link

⚠️ This PR changed pyproject.toml file. ⚠️

  • PR Creator must update 📃 THIRD_PARTY_LICENSES.txt, if any 📚 library added/removed in pyproject.toml.
  • PR Approver must confirm 📃 THIRD_PARTY_LICENSES.txt updated, if any 📚 library added/removed in pyproject.toml.

@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

@github-actions
Copy link

⚠️ This PR changed pyproject.toml file. ⚠️

  • PR Creator must update 📃 THIRD_PARTY_LICENSES.txt, if any 📚 library added/removed in pyproject.toml.
  • PR Approver must confirm 📃 THIRD_PARTY_LICENSES.txt updated, if any 📚 library added/removed in pyproject.toml.

1 similar comment
@github-actions
Copy link

⚠️ This PR changed pyproject.toml file. ⚠️

  • PR Creator must update 📃 THIRD_PARTY_LICENSES.txt, if any 📚 library added/removed in pyproject.toml.
  • PR Approver must confirm 📃 THIRD_PARTY_LICENSES.txt updated, if any 📚 library added/removed in pyproject.toml.

@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

mrDzurb
mrDzurb previously approved these changes Sep 20, 2023

# Add default signer if the uri is an object storage uri, and
# the user does not specify config or signer.
if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to check on the "oci://"?

Copy link
Member Author

@liudmylaru liudmylaru Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied lines 94-99 (https://github.com/oracle/accelerated-data-science/blob/main/ads/jobs/serializer.py#L94-L99) from the same file, I think QQ told me he forgot to add this code here also. So my answer to your question - I do not know :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the uri does not start with oci://, the config and signer are not needed. Also we don't want to send the credentials to non-oci server (which could be a security issue).

@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- removed hardcoded config path from test_dsc_job.py
- added if to update kwargs with default_signer in jobs  serializer write_to_fle
- fixed check-docstring-first pro-commit check in serializer.py
- fix test_jobs_cli.py
- fix test_jobs_notebook.py
- fix test_jobs_cli.py
- fix test_jobs_runs.py
- updated driver_utils.py to work with Instance Principals
- added default_signer() into test_notebook_driver_with_outputs()
@liudmylaru liudmylaru force-pushed the ODSC-47592/fix_int_test_jobs_python_runtime branch from 5b1dd56 to 969e15d Compare November 7, 2023 22:25
@github-actions
Copy link

github-actions bot commented Nov 7, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- set OCI_IAM_TYPE env var for run driver in jobs tests
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- fix typo in test_jobs_cli.py
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- updated tests in "other" folder
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- rollback changes in driver_utils.py
- fix test_jobs_cli.py
- fix test_artifact_saving_data.py
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

@liudmylaru liudmylaru changed the title Fix test_jobs_python_runtime.py Fix integration tests to run with instance_principal Nov 8, 2023
- update env_vars to run_driver()
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- mark test_notebook_driver_with_outputs() skipped
@github-actions
Copy link

github-actions bot commented Nov 8, 2023

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

- fixes to OPCTL int tests
- marked skipped one data_flow test
@github-actions
Copy link

📌 Cov diff with main:

No success to gather report. 😿

📌 Overall coverage:

No success to gather report. 😿

@liudmylaru liudmylaru requested a review from mrDzurb November 13, 2023 20:34
@github-actions
Copy link

📌 Cov diff with main:

Coverage-50%

📌 Overall coverage:

Coverage-71.49%


# Add default signer if the uri is an object storage uri, and
# the user does not specify config or signer.
if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the uri does not start with oci://, the config and signer are not needed. Also we don't want to send the credentials to non-oci server (which could be a security issue).

@liudmylaru liudmylaru merged commit c8ed767 into main Nov 14, 2023
@liudmylaru liudmylaru deleted the ODSC-47592/fix_int_test_jobs_python_runtime branch November 14, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants