Skip to content

Commit

Permalink
Explicitly add all public ops.X names to __all__ in __init__.py (#937)
Browse files Browse the repository at this point in the history 
This is so that when using "import ops; ops.X" in a charm, Pyright
and MyPy won't complain (in the charm's codebase).

For example, MyPy:

src/charm.py:14: error: Name "ops.CharmBase" is not defined  [name-defined]

And Pyright:

src/charm.py:32:50 - error: "CharmBase" is not exported from module "ops" (reportPrivateImportUsage)

This also avoids disabling the reportUnusedImport setting.

I was considering adding a test that checked that __all__ stays in sync
with the imports below, but Pyright errors if we have a entry in one
but not the other.

Also:

* Remove submodules from docs to avoid duplicate entries
* A few other docs fixes/simplifications
* Sort docs alphabetically. This makes things easier to find.
  If you want source order, read the source.
  • Loading branch information
@benhoyt
benhoyt authored Jun 1, 2023
1 parent 74e7a5c commit cde9900
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 84 deletions.
6 changes: 3 additions & 3 deletions docs/conf.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
# Add any paths that contain custom static files (such as style sheets) here,
# relative to this directory. They are copied after the builtin static files,
# so a file named "default.css" will overwrite the builtin "default.css".
html_static_path = ['_static']
html_static_path = []


# -- Options for sphinx.ext.todo ---------------------------------------------
Expand All @@ -98,13 +98,13 @@
# 'both' - Both the class’ and the __init__ method’s docstring are
# concatenated and inserted.
# 'init' - Only the __init__ method’s docstring is inserted.
autoclass_content = 'both'
autoclass_content = 'class'

# This value selects if automatically documented members are sorted
# alphabetical (value 'alphabetical'), by member type (value
# 'groupwise') or by source order (value 'bysource'). The default is
# alphabetical.
autodoc_member_order = 'bysource'
autodoc_member_order = 'alphabetical'

autodoc_default_options = {
'members': None, # None here means "yes"
Expand Down
33 changes: 0 additions & 33 deletions docs/index.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,6 @@ ops package

.. automodule:: ops

Submodules
----------

ops.charm module
----------------

.. automodule:: ops.charm

ops.framework module
--------------------

.. automodule:: ops.framework

ops.jujuversion module
----------------------

.. automodule:: ops.jujuversion

ops.log module
--------------

.. automodule:: ops.log

ops.main module
---------------

.. automodule:: ops.main

ops.model module
----------------

.. automodule:: ops.model


ops.pebble module
-----------------
Expand Down
123 changes: 119 additions & 4 deletions ops/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,129 @@
Full developer documentation is available at https://juju.is/docs/sdk.
"""

# The "from .X import Y" imports below don't explicitly tell Pyright (or MyPy)
# that those symbols are part of the public API, so we have to add __all__.
__all__ = [
'__version__',
'main',
'pebble',

# From charm.py
'ActionEvent',
'ActionMeta',
'CharmBase',
'CharmEvents',
'CharmMeta',
'CollectMetricsEvent',
'ConfigChangedEvent',
'ContainerMeta',
'ContainerStorageMeta',
'HookEvent',
'InstallEvent',
'LeaderElectedEvent',
'LeaderSettingsChangedEvent',
'PayloadMeta',
'PebbleReadyEvent',
'PostSeriesUpgradeEvent',
'PreSeriesUpgradeEvent',
'RelationBrokenEvent',
'RelationChangedEvent',
'RelationCreatedEvent',
'RelationDepartedEvent',
'RelationEvent',
'RelationJoinedEvent',
'RelationMeta',
'RelationRole',
'RemoveEvent',
'ResourceMeta',
'SecretChangedEvent',
'SecretEvent',
'SecretExpiredEvent',
'SecretRemoveEvent',
'SecretRotateEvent',
'StartEvent',
'StopEvent',
'StorageAttachedEvent',
'StorageDetachingEvent',
'StorageEvent',
'StorageMeta',
'UpdateStatusEvent',
'UpgradeCharmEvent',
'WorkloadEvent',

# From framework.py
'BoundEvent',
'BoundStoredState',
'CommitEvent',
'EventBase',
'EventSource',
'Framework',
'FrameworkEvents',
'Handle',
'HandleKind',
'LifecycleEvent',
'NoTypeError',
'Object',
'ObjectEvents',
'PreCommitEvent',
'PrefixedEvents',
'StoredDict',
'StoredList',
'StoredSet',
'StoredState',
'StoredStateData',

# From jujuversion.py
'JujuVersion',

# From model.py
'ActiveStatus',
'Application',
'Binding',
'BindingMapping',
'BlockedStatus',
'CheckInfoMapping',
'ConfigData',
'Container',
'ContainerMapping',
'ErrorStatus',
'InvalidStatusError',
'LazyMapping',
'MaintenanceStatus',
'Model',
'ModelError',
'MultiPushPullError',
'Network',
'NetworkInterface',
'OpenedPort',
'Pod',
'Relation',
'RelationData',
'RelationDataAccessError',
'RelationDataContent',
'RelationDataError',
'RelationDataTypeError',
'RelationMapping',
'RelationNotFoundError',
'Resources',
'Secret',
'SecretInfo',
'SecretNotFoundError',
'SecretRotate',
'ServiceInfoMapping',
'StatusBase',
'Storage',
'StorageMapping',
'TooManyRelatedAppsError',
'Unit',
'UnknownStatus',
'WaitingStatus',
]

# The isort command wants to rearrange the nicely-formatted imports below;
# just skip it for this file.
# isort:skip_file

# Similarly, Pyright complains that all of these things are unused imports,
# so disable it:
# pyright: reportUnusedImport=false

# Import pebble explicitly. It's the one module we don't import names from below.
from . import pebble # type: ignore # noqa: F401

Expand Down
61 changes: 32 additions & 29 deletions ops/charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,30 +189,35 @@ def restore(self, snapshot: 'JsonObject'):
def set_results(self, results: '_SerializedData'):
"""Report the result of the action.
Args:
results: The result of the action as a Dict
Juju eventually only accepts a str:str mapping, so we will attempt
to flatten any more complex data structure like so:
Juju eventually only accepts a str:str mapping, so we will attempt
to flatten any more complex data structure like so::
>>> {'a': 'b'} # becomes: 'a'='b'
>>> {'a': {'b': 'c'}} # becomes: 'a.b'='c'
>>> {'a': {'b': 'c', 'd': 'e'}} # becomes: 'a.b'='c', 'a.d' = 'e'
>>> {'a.b': 'c', 'a.d': 'e'} # equivalent to previous
Note that duplicate keys are not allowed, so
>>> {'a': {'b': 'c'}, 'a.b': 'c'} # invalid!
Note that the resulting keys must start and end with lowercase
alphanumeric, and can only contain lowercase alphanumeric, hyphens
and periods.
If any exceptions occur whilst the action is being handled, juju will
gather any stdout/stderr data (and the return code) and inject them into the
results object. Thus, the results object might contain the following keys,
additionally to those specified by the charm code:
- Stdout
- Stderr
- Stdout-encoding
- Stderr-encoding
- ReturnCode
Note that duplicate keys are not allowed, so this is invalid::
>>> {'a': {'b': 'c'}, 'a.b': 'c'}
Note that the resulting keys must start and end with lowercase
alphanumeric, and can only contain lowercase alphanumeric, hyphens
and periods.
If any exceptions occur whilst the action is being handled, juju will
gather any stdout/stderr data (and the return code) and inject them into the
results object. Thus, the results object might contain the following keys,
additionally to those specified by the charm code:
- Stdout
- Stderr
- Stdout-encoding
- Stderr-encoding
- ReturnCode
Args:
results: The result of the action as a Dict
"""
self.framework.model._backend.action_set(results) # pyright: reportPrivateUsage=false

Expand Down Expand Up @@ -545,11 +550,6 @@ class RelationDepartedEvent(RelationEvent):
Once all callback methods bound to this event have been run for such a
relation, the unit agent will fire the :class:`RelationBrokenEvent`.
Attributes:
departing_unit: The :class:`~ops.model.Unit` that is departing. This
can facilitate determining e.g. whether *you* are the departing
unit.
"""

def __init__(self, handle: 'Handle', relation: 'Relation',
Expand All @@ -572,7 +572,11 @@ def snapshot(self) -> '_RelationDepartedEventSnapshot':

@property
def departing_unit(self) -> Optional[model.Unit]:
"""The `ops.model.Unit` that is departing, if any."""
"""The :class:`ops.Unit` that is departing, if any.
You can use this to determine (for example) whether *you* are the
departing unit.
"""
# doing this on init would fail because `framework` gets patched in
# post-init
if not self._departing_unit_name:
Expand Down Expand Up @@ -955,7 +959,7 @@ def __init__(self, *args):
main(MyCharm)
As shown in the example above, a charm class is instantiated by
:func:`~ops.main.main` rather than charm authors directly instantiating a
:code:`ops.main` rather than charm authors directly instantiating a
charm.
Args:
Expand Down Expand Up @@ -1279,7 +1283,6 @@ class ContainerMeta:
Attributes:
name: Name of container (key in the YAML)
mounts: :class:`ContainerStorageMeta` mounts available to the container
"""

def __init__(self, name: str, raw: '_ContainerMetaDict'):
Expand Down Expand Up @@ -1337,7 +1340,6 @@ class ContainerStorageMeta:
storage: a name for the mountpoint, which should exist the keys for :class:`StorageMeta`
for the charm
location: the location `storage` is mounted at
locations: a list of mountpoints for the key
If multiple locations are specified for the same storage, such as Kubernetes subPath mounts,
`location` will not be an accessible attribute, as it would not be possible to determine
Expand All @@ -1358,6 +1360,7 @@ def locations(self) -> List[str]:
return self._locations

def __getattr__(self, name: str):
# TODO(benhoyt): this should just be a property "location"
if name == "location":
if len(self._locations) == 1:
return self._locations[0]
Expand Down
19 changes: 10 additions & 9 deletions ops/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1740,7 +1740,6 @@ class Storage:
Attributes:
name: Simple string name of the storage
index: The index number for storage
"""

def __init__(self, storage_name: str, storage_index: int, backend: '_ModelBackend'):
Expand Down Expand Up @@ -1784,16 +1783,18 @@ def location(self, location: str) -> None:


class MultiPushPullError(Exception):
"""Aggregates multiple push/pull related exceptions into one."""
"""Aggregates multiple push/pull related exceptions into one.
def __init__(self, message: str, errors: List[Tuple[str, Exception]]):
"""Create an aggregation of several push/pull errors.
This class should not be instantiated directly.
Args:
message: error message
errors: list of errors with each represented by a tuple (<source_path>,<exception>)
where source_path is the path being pushed/pulled from.
"""
Attributes:
message: error message
errors: list of errors with each represented by a tuple (<source_path>,<exception>)
where source_path is the path being pushed/pulled from.
"""

def __init__(self, message: str, errors: List[Tuple[str, Exception]]):
"""Create an aggregation of several push/pull errors."""
self.errors = errors
self.message = message

Expand Down
11 changes: 5 additions & 6 deletions ops/pebble.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1403,19 +1403,18 @@ def read1(self, n: int = -1) -> '_StrOrBytes':


class Client:
"""Pebble API client."""
"""Pebble API client.
Defaults to using a Unix socket at socket_path (which must be specified
unless a custom opener is provided).
"""

_chunk_size = 8192

def __init__(self, socket_path: str,
opener: Optional[urllib.request.OpenerDirector] = None,
base_url: str = 'http://localhost',
timeout: float = 5.0):
"""Initialize a client instance.
Defaults to using a Unix socket at socket_path (which must be specified
unless a custom opener is provided).
"""
if not isinstance(socket_path, str):
raise TypeError(f'`socket_path` should be a string, not: {type(socket_path)}')
if opener is None:
Expand Down

0 comments on commit cde9900

Please sign in to comment.