Operator Framework charm writing guide #135
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TODO: Migrating from old charm framework, Useful links
Currently .app property is None when a peer relation contains only one unit (relation-list returns an empty list). This change addresses the issue.
Hook tool handling for network-get which may be a base for further endpoint modeling in the model.
Added Framework events doc, continued charm writing docs
…ical#124) * Add a makefile showing how to run lint and tests, and fix lint * Set default make target as running lint and tests, refactor _less_than_property for clarity * Include autopep8 in lint target and update travis.yml to use Makefile * Revert to pre-existing code for __lt__ in jujuversion, and explicitly specify flake8 config location * Add missing return line in __lt__ for jujuversion
relaxdiego
reviewed
Feb 11, 2020
Added testing.md, addressed review comments
|
Thank you for this! I'll be doing a deeper review as soon as my time permits. For now, some notes:
|
|
Hey @chipaca thank you for the feedback. Sure will take on all those points, the docs were very much written on a whim, and we just kept rolling with it. Will address your points in the next commit. Cheers! |
* Removed CamelCase * Fixed informal language (incomplete) * Added text back into README
VariableDeclared
changed the title
VariableDeclared
changed the title
Python has a rich enough variety of quotes that you should never need to use a backslash to protect a single quote in a string; it a silly little thing but it hurts readability. So this fixes that, and adds a linter to make sure it stays fixed. Modulo bugs in the linter of course.
Rework handling of exclusive db access * sqlite3 Python module provides an implicit transaction management functionality when isolation_level argument is not None when passed to a connect method; * sqlite3 itself by default operates in the autocommit mode which can be disabled by executing the "BEGIN" SQL command; https://www.sqlite.org/lockingv3.html#transaction_control * BEGIN without EXCLUSIVE does not obtain an exclusive lock for the database and the default behavior is to run a transaction as DEFERRED; https://www.sqlite.org/lang_transaction.html#immediate The desired behavior for the framework is holding an exclusive access to the DB while a Framework object exists and is not closed which requires maintaining exclusive access across transactions. This is possible to achieve by using `PRAGMA locking_mode=EXCLUSIVE`. With this change, it will not be possible to have two Framework objects with exclusive access to the backend storage.
Update all the key files with the license header, and add the LICENSE.txt explaining this is Apache 2.0.
…onical#138) This aligns ActiveStatus with the other constructors (which all accept a status message argument) and makes it easier to treat all subclasses of StatusBase in a first-class way. The current behaviour of using an empty message string when no argument is given is preserved. NOTE the plan is to eventually make the need for this disappear: > As was extensively discussed in canonical#126, the fact it has no > message is not a mistake. It was done on purpose because we do > intend for that message to indeed go away from juju entirely > for the reasons described there. That said, as described there > as well, people do see value in the message today and the > alternative isn't there yet. So as a solution I suggest > accepting this, allowing the message in the framework, then > adding support for structured information in juju itself, and > finally dropping the displaying (reading) of that message in > juju proper so that the message becomes irrelevant. In that > order.
relaxdiego
reviewed
Feb 18, 2020
| @@ -0,0 +1,27 @@ | |||
| # Framework events | |||
There was a problem hiding this comment.
I wonder if it's better to link most of these events to this page instead: https://discourse.jujucharms.com/t/charm-hooks/1040
There was a problem hiding this comment.
So this raises a good point. One of the goals I wanted to address with this PR is the fragmented nature of docs.
It seems that docs for charms are here, and there. I was hoping for a single source of truth for all things writing charms.
* Add a check that all non-empty .py files have a Copyright header.
* add python 3.8 testing, and also test on arm * further tweak .travis.yml (logs are nicer with this) * add python 3.8 to list of supported, now that we run tests on it
* Added network bindings that can be looked up by relation-names, extra-binding names or Relation objects; * Bindings can be looked up by relations to handle the cross-model relations logic of populating ingress-address and engres-subnets values depending on a relation ID: https://jaas.ai/docs/charm-network-primitives https://github.com/juju/juju/blob/juju-2.6.7/state/relationunit.go#L634-L658 * `binding.network` represents a view of the data available via a bound network space; * `binding.network.interfaces` represents all logical network interfaces (multiple addresses per network device are flattened into a single list of NetworkInterfaces); * L3 addresses and subnets are exposed via their respective types in the ipaddress module; * hostnames are not yet exposed as their semantics are not clear yet, see https://bugs.launchpad.net/juju/+bug/1864086 * mac addresses are specific to ethernet so they are also not yet exposed yet, see https://bugs.launchpad.net/juju/+bug/1864070.
* log levels for the root logger and a JujuLogHandler instance added in main.py are set to DEBUG to pass all messages to the controller where they can be filtered; * it might be that charms need a way to retrieve a log level set for a unit configured at the model level, however, this is not possible at the moment (see LP: #1861987); * TRACE level supported by Juju is not present in Python, thus it is ignored for the purposes of this PR.
ghost
reviewed
Feb 25, 2020
docs/charmsindetail.md
Outdated
|
|
||
| ## Best practices | ||
|
|
||
| ### Repoository Naming |
* add CollectMetricsEvent and a relevant API for supporting add-metric hook tool on that event; * add CollectMetricsEvent to CharmEvents; * At the time of writing, collect-metrics hooks can run concurrently to other hooks ignoring the usual semantics, however, this is serialized via DB locking with a long timeout; * collect-metrics hooks run in a limited environment where regular hook tools are not available; * Utility of deferring collect-metrics hooks is questionable but this is not prevented; * metrics keys, metric values, label keys and label values are sanity-checked, although those restrictions are not yet in place at the time of writing.
* Allow binding lookup without relation ids * Networking information associated with a relation endpoint can be useful before the relations themselves exist; * It is not possible to retrieve networking info for dead relations so for those cases networking info is looked up as if a relation ID was not specified.
Add at least basic documentation for all of the Charm public API.
* Use Unicode characters for tree output Improve how the skeleton structure looks by moving away from ASCII-only characters.
…dependencies and how to test. (canonical#184) * Moved all tests to unit tests, removed Makefile, improved README for dependencies and how to test. * Run autopep8 ONLY on those files reported by flake8.
Add an `ops.testing.Harness` class that can be used when testing your Charm.
Example test case:
```
from ops.testing import Harness
class TestMyCharm(unittest.TestCase):
def setUp(self):
self.harness = Harness(MyCharm)
def test_relation_changed(self):
# Initial setup of the Model
rel_id = self.harness.add_relation('foo', 'foo-provider')
self.harness.add_relation_unit(rel_id, 'foo-provider/0', remote_unit_data={'foo': 'bar'})
# Instantiate the charm, and respond to hook events
self.harness.begin()
self.harness.update_relation(rel_id, 'foo-provider/0', {'foo': 'baz'})
self.assertEqual(self.harness.charm.state.foo, 'baz')
```
Before `begin()` you can set up a Model to be in the desired precondition state (what relations exist, etc). After `begin()` updates to the harness will trigger the associated events (eg `update_relation` triggers `relation-changed`.) And you can check whether your Charm properly handles the event.
* Renamed EventsBase to EventSetBase (and derivatives). * Removed exec bit.
…nonical#199) Before this change, if you create a class that uses StoredState, e.g. class MyClass(Object): _stored = StoredState() and then subclassed it, class Another(MyClass): pass things broke. This fixes that.
…al#194. (canonical#195) * Test event marshalling, and improved its error message. Co-authored-by: John R. Lenton <jlenton@gmail.com>
…nonical#207) People will look at our tests for inspiration about how to do things, even if we had comprehensive documentation (which we currently don't). This makes them a little better on this front.
"remove" is fired after stop when a unit is removed. juju/juju#11237
TODO: Migrating from old charm framework, Useful links
Added Framework events doc, continued charm writing docs
Added testing.md, addressed review comments
* Removed CamelCase * Fixed informal language (incomplete) * Added text back into README
|
|
||
| ### Add interface dependences | ||
|
|
||
| To begin a new charm we should take note of an Interfaces that might be needed, the operator framework uses Interfaces as dependencies. |
There was a problem hiding this comment.
Possible typo here for "any Interfaces"?
Signed-off-by: Peter J De Sousa <peter.de.sousa@canonical.com>
|
Again, thanks for working on this documentation! I'm closing this PR, but only because this repository is not the proper place for this documentation to live. @chipaca We need to work with @VariableDeclared so that this is moved to the proper place and not lost here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Docs for those getting started with the new framework.