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.

Sign up for GitHub

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

docs: use 'integrate with' rather than 'relate to' #1145

Merged
merged 3 commits into from
Mar 8, 2024
Merged

docs: use 'integrate with' rather than 'relate to' #1145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47a9f8a
Use 'integrate with' rather than 'relate to.
tonyandrewmeyer Mar 8, 2024
19cf1bf
Merge branch 'main' into integrate-not-relate-1133
tonyandrewmeyer Mar 8, 2024
c0244ee
Update ops/testing.py
tonyandrewmeyer Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions ops/charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,10 @@ def restore(self, snapshot: Dict[str, Any]):
class RelationCreatedEvent(RelationEvent):
"""Event triggered when a new relation is created.

This is triggered when a new relation to another app is added in Juju. This
This is triggered when a new integration with another app is added in Juju. This
can occur before units for those applications have started. All existing
relations should be established before start.
relations will trigger `RelationCreatedEvent` before :class:`StartEvent` is
emitted.
Comment on lines +512 to +513
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a drive-by: the existing wording is quite unclear (to me, at least). I think the revised wording is what is meant?

"""
unit: None # pyright: ignore[reportIncompatibleVariableOverride]
"""Always ``None``."""
Expand All @@ -521,7 +522,7 @@ class RelationJoinedEvent(RelationEvent):
This event is triggered whenever a new unit of a related
application joins the relation. The event fires only when that
remote unit is first observed by the unit. Callback methods bound
to this event may set any local unit settings that can be
to this event may set any local unit data that can be
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This (and the below) are another drive-by: I only recall seeing "settings" meaning the content of the relation databag in the oldest documentation, and everywhere else in the API docs we refer to "data", so I believe this is much clearer.

determined using no more than the name of the joining unit and the
remote ``private-address`` setting, which is always available when
the relation is created and is by convention not deleted.
Expand All @@ -539,13 +540,13 @@ class RelationChangedEvent(RelationEvent):
the callback method bound to this event.

This event always fires once, after :class:`RelationJoinedEvent`, and
will subsequently fire whenever that remote unit changes its settings for
will subsequently fire whenever that remote unit changes its data for
the relation. Callback methods bound to this event should be the only ones
that rely on remote relation settings. They should not error if the settings
are incomplete, since it can be guaranteed that when the remote unit or
application changes its settings, the event will fire again.
that rely on remote relation data. They should not error if the data
is incomplete, since it can be guaranteed that when the remote unit or
application changes its data, the event will fire again.

The settings that may be queried, or set, are determined by the relation's
The data that may be queried, or set, are determined by the relation's
interface.
"""

Expand Down
16 changes: 8 additions & 8 deletions ops/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def app(self) -> 'Application':
def relations(self) -> 'RelationMapping':
"""Mapping of endpoint to list of :class:`Relation`.

Answers the question "what am I currently related to".
Answers the question "what am I currently integrated with".
See also :meth:`.get_relation`.

In a ``relation-broken`` event, the broken relation is excluded from
Expand Down Expand Up @@ -236,7 +236,7 @@ def get_relation(
given application has more than one relation on a given endpoint.

Raises:
TooManyRelatedAppsError: is raised if there is more than one relation to the
TooManyRelatedAppsError: is raised if there is more than one integration with the
supplied relation_name and no relation_id was supplied
"""
return self.relations._get_unique(relation_name, relation_id)
Expand Down Expand Up @@ -315,8 +315,8 @@ def get(self, entity_type: 'UnitOrApplicationType', name: str):
class Application:
"""Represents a named application in the model.

This might be this charm's application, or might be an application this charm is related
to. Charmers should not instantiate Application objects directly, but should use
This might be this charm's application, or might be an application this charm is integrated
with. Charmers should not instantiate Application objects directly, but should use
:attr:`Model.app` to get the application this unit is part of, or
:meth:`Model.get_app` if they need a reference to a given application.
"""
Expand Down Expand Up @@ -472,7 +472,7 @@ class Unit:
"""Represents a named unit in the model.

This might be the current unit, another unit of the charm's application, or a unit of
another application that the charm is related to.
another application that the charm is integrated with.
"""

name: str
Expand Down Expand Up @@ -1865,8 +1865,8 @@ class MaintenanceStatus(StatusBase):
class WaitingStatus(StatusBase):
"""A unit is unable to progress.

The unit is unable to progress to an active state because an application to which
it is related is not running.
The unit is unable to progress to an active state because an application with which
it is integrated is not running.
Comment on lines +1868 to +1869
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benhoyt this seems very specific about the use-case for WaitingStatus. Isn't it ok to use it if you're waiting on anything, rather than specifically for an integration/relation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's taken straight from the comment in Juju core/status/status.go, so I presume it's intentional:

 	// The unit is unable to progress to an active state because an application to
	// which it is related is not running.

I think you'd use MaintenanceStatus otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's taken straight from the comment in Juju core/status/status.go, so I presume it's intentional:

Ah, ok. It should presumably stay in that case.

I think you'd use MaintenanceStatus otherwise.

Hmm, if I'm waiting for the container to be ready or waiting for an API to come up, WaitingStatus seems the natural one to use. MaintenanceStatus seems like it's more when the charm is actively doing something.


"""
name = 'waiting'
Expand Down Expand Up @@ -2856,7 +2856,7 @@ class ModelError(Exception):


class TooManyRelatedAppsError(ModelError):
"""Raised by :meth:`Model.get_relation` if there is more than one related application."""
"""Raised by :meth:`Model.get_relation` if there is more than one integrated application."""

def __init__(self, relation_name: str, num_related: int, max_supported: int):
super().__init__('Too many remote applications on {} ({} > {})'.format(
Expand Down
8 changes: 4 additions & 4 deletions ops/testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ def remove_storage(self, storage_id: str) -> None:
def add_relation(self, relation_name: str, remote_app: str, *,
app_data: Optional[Mapping[str, str]] = None,
unit_data: Optional[Mapping[str, str]] = None) -> int:
"""Declare that there is a new relation between this application and `remote_app`.
"""Declare that there is a new integration between this application and `remote_app`.
tonyandrewmeyer marked this conversation as resolved.
Show resolved Hide resolved

This function creates a relation with an application and triggers a
:class:`RelationCreatedEvent <ops.RelationCreatedEvent>`.
Expand All @@ -819,8 +819,8 @@ def add_relation(self, relation_name: str, remote_app: str, *,
})

Args:
relation_name: The relation on the charm that is being related to.
remote_app: The name of the application that is being related to.
relation_name: The relation on the charm that is being integrated with.
remote_app: The name of the application that is being integrated with.
To add a peer relation, set to the name of *this* application.
app_data: If provided, also add a new unit to the relation
(triggering relation-joined) and set the *application* relation data
Expand Down Expand Up @@ -1339,7 +1339,7 @@ def set_leader(self, is_leader: bool = True) -> None:

If this charm becomes a leader then `leader_elected` will be triggered. If :meth:`begin`
has already been called, then the charm's peer relation should usually be added *prior* to
calling this method (with :meth:`add_relation`) to properly initialize and make
calling this method (with :meth:`add_relation`) to properly initialise and make
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One last wee drive-by.

available relation data that leader elected hooks may want to access.

Args:
Expand Down
Loading