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

Add jupyter-idle extension #453

Merged
merged 4 commits into from
Jul 11, 2024
Merged

Add jupyter-idle extension #453

merged 4 commits into from
Jul 11, 2024

Conversation

rahrad123
Copy link
Contributor

@rahrad123 rahrad123 commented Jul 8, 2024

Issue #, if available:

  • Was unable to get all tests to pass, some tests which have been added previously has been failing. Ex : mlflow.test.Dockerfile

Description of changes:

Test result

dev-dsk-rahrad-2a-4fa8580c % sudo python -m pytest -n auto -m cpu -vv -rs -k "jupyter-activity-monitor-extension.test.Dockerfile" --local-image-version 2.0.0
==================================================================================================================== test session starts =====================================================================================================================
platform linux -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0 -- /home/rahrad/miniforge3/envs/sagemaker-distribution/bin/python
cachedir: .pytest_cache
rootdir: /local/home/rahrad/sagemaker-distribution
configfile: pytest.ini
plugins: anyio-4.4.0, xdist-3.6.1, mock-3.14.0
8 workers [1 item]      
scheduling tests via LoadScheduling

test/test_dockerfile_based_harness.py::test_dockerfiles_for_cpu[jupyter-activity-monitor-extension.test.Dockerfile-required_packages28] 
[gw0] [100%] PASSED test/test_dockerfile_based_harness.py::test_dockerfiles_for_cpu[jupyter-activity-monitor-extension.test.Dockerfile-required_packages28] 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

- Added the package to the {cpu/gpu}.additional_packages_env.in files
- Added import test for the jupyter-idle extension package
- Built the images successfully and tested the extension on jupyter lab
  spun up on the image. Results were as expected.
- An OSS review and Appsec review has been copleted for this package
- pypi : https://pypi.org/project/jupyter-idle/0.1.1/
- conda-forge : https://anaconda.org/conda-forge/jupyter-idle

ARG MAMBA_DOCKERFILE_ACTIVATE=1

CMD ["python", "-c", "import jupyter_idle"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add better tests than this.
I'm assuming that your extension returns a json response. can we at least validate whether the response has the correct keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add a functional test as a follow up later in the week.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new commit with unit test changes

balajisankar15
balajisankar15 previously approved these changes Jul 10, 2024
claytonparnell
claytonparnell previously approved these changes Jul 11, 2024
@claytonparnell claytonparnell dismissed stale reviews from balajisankar15 and themself via b6140fe July 11, 2024 16:55
@claytonparnell claytonparnell merged commit 99db206 into aws:main Jul 11, 2024
1 check passed
rahrad123 added a commit to rahrad123/sagemaker-distribution that referenced this pull request Jul 22, 2024
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

Successfully merging this pull request may close these issues.

3 participants