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

switch code quality workflow to dev target and smoketest #2032

Merged
merged 10 commits into from
Mar 4, 2023

Conversation

dakinggg
Copy link
Contributor

@dakinggg dakinggg commented Mar 3, 2023

What does this PR do?

This PR does two things:

  • switches our code quality workflow back to dev target to avoid forcing install all for linting to pass
  • adds a smoketest workflow that just imports the top level composer modules to avoid accidentally missing conditional imports

What issue(s) does this change relate to?

Closes CO-1866

Before submitting

  • Have you read the contributor guidelines?
  • Was this change discussed/approved in a GitHub issue first? It is much more likely to be merged if so.
  • Did you update any related docs and document your change?
  • Did you update any related tests and add any new tests related to your change? (see testing)
  • Did you run the tests locally to make sure they pass?
  • Did you run pre-commit on your change? (see the pre-commit section of prerequisites)

@dakinggg dakinggg changed the title switch code quality workflow to dev target switch code quality workflow to dev target and smoketest Mar 3, 2023
@dakinggg dakinggg mentioned this pull request Mar 3, 2023
7 tasks
@dakinggg dakinggg marked this pull request as ready for review March 3, 2023 23:57
@dakinggg dakinggg requested review from a team as code owners March 3, 2023 23:57
@dakinggg
Copy link
Contributor Author

dakinggg commented Mar 4, 2023

Before the import fix from hanlin's pr, running the smoketest locally

(composer-dev-3.10) danielking@MML-1B940F4333E2 composer % pytest tests/test_smoketest.py 
ImportError while loading conftest '/Users/danielking/github/composer/tests/conftest.py'.
tests/conftest.py:9: in <module>
    from composer.utils import reproducibility
composer/__init__.py:10: in <module>
    from composer.trainer import Trainer
composer/trainer/__init__.py:6: in <module>
    from composer.trainer.trainer import Trainer
composer/trainer/trainer.py:33: in <module>
    from composer.callbacks import CheckpointSaver, OptimizerMonitor
composer/callbacks/__init__.py:12: in <module>
    from composer.callbacks.health_checker import HealthChecker
composer/callbacks/health_checker.py:18: in <module>
    import slack_sdk
E   ModuleNotFoundError: No module named 'slack_sdk'

after

(composer-dev-3.10) danielking@MML-1B940F4333E2 composer % pytest tests/test_smoketest.py
=================================================================================== test session starts ===================================================================================
platform darwin -- Python 3.10.9, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/danielking/github/composer, configfile: pyproject.toml
plugins: pytest_codeblocks-0.16.1, anyio-3.6.2, httpserver-1.0.6
collected 1 item                                                                                                                                                                          

tests/test_smoketest.py .                                                                                                                                                           [100%]

==================================================================================== 1 passed in 0.02s ====================================================================================
(composer-dev-3.10) danielking@MML-1B940F4333E2 composer % 

Copy link
Contributor

@bandish-shah bandish-shah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding this simple but likely very useful sanity check.

tests/test_smoketest.py Outdated Show resolved Hide resolved
.github/workflows/smoketest.yaml Show resolved Hide resolved
Co-authored-by: Mihir Patel <mihir.v.patel7@gmail.com>
@dakinggg dakinggg enabled auto-merge (squash) March 4, 2023 00:55
@dakinggg dakinggg merged commit dc5ab48 into mosaicml:dev Mar 4, 2023
@dakinggg dakinggg deleted the dev-install-target branch June 1, 2023 00:14
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