-
Notifications
You must be signed in to change notification settings - Fork 17
chore: reenable ops.main type hints #298
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
chore: reenable ops.main type hints #298
Conversation
|
P.S. can be tested locally with: diff --git a/requirements.txt b/requirements.txt
index 0eeaba6..c8a95c5 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,6 @@
cosl
distro
-ops >= 2.17.0
+git+https://github.com/canonical/operator.git@main#egg=ops
jinja2
redfish # requests is included in this
pydantic < 2 |
|
Hi @dimaqq I converted this PR to draft until 2.17 is not released, then we can convert it back and re-trigger CI. |
|
ops 2.17.0 is released, so I retriggered the CI and marked the PR as ready for review :) |
|
Can't we unpin the ops library? |
|
@dimaqq sorry, the CI is blocked on commits needing to be signed - could you gpg sign your commits (you can squash and force push to make it easier)? |
58700d3 to
9a137e0
Compare
|
Sorry I forgot to force-push earlier, here it is. |
bb6feab to
ef0a904
Compare
|
re the other CI fail, I'll be releasing a hotfix for pypi:juju today 🙇🏻 |
ef0a904 to
9d33835
Compare
|
The test fail is a mystery to me now. |
|
Hmm it looks like I already added the fix in another PR to fix the lint failure: #330 . So there arent any new changes in this PR apart from the temporary change to test with ops on main. I think we should simply close this PR without merging. Sorry for the confusion. |
Type hints are finally fixed for
ops.main()call in the upcoming ops release.The mypy config includes
warn_unused_ignoresin this repo, which means that using new ops will trigger an error soon.P.S. can only be merged when ops 2.17.0 is released.