From 9c2f5d706c61859e69dd1b7a3b54f898dcf8d480 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 28 May 2019 22:31:12 +0200
Subject: Contribution: split source into smaller files

---
 docs/contributing/coding_guidelines.rst | 144 ++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)
 create mode 100644 docs/contributing/coding_guidelines.rst

(limited to 'docs/contributing/coding_guidelines.rst')

diff --git a/docs/contributing/coding_guidelines.rst b/docs/contributing/coding_guidelines.rst
new file mode 100644
index 00000000..ff863d61
--- /dev/null
+++ b/docs/contributing/coding_guidelines.rst
@@ -0,0 +1,144 @@
+.. _coding_guidelines:
+
+Python Coding Guidelines
+========================
+
+The switch to the Python programming language for new code is not merely a change
+of the language, but a chance to rethink and improve the programming approach.
+
+Let's face it: VyOS is full of spaghetti code where logic for reading the VyOS
+config, generating daemon configs, and restarting processes is all mixed up.
+
+Python (or any other language, for that matter) does not provide automatic protection
+from bad design, so we need to also devise design guidelines and follow them to
+keep the system extensible and maintainable.
+
+Configuration script structure and behaviour
+--------------------------------------------
+
+.. code-block:: python
+
+  import sys
+
+  from vyos.config import Config
+  from vyos.util import ConfigError
+
+  def get_config():
+      vc = Config()
+      # Convert the VyOS config to an abstract internal representation
+      config = ...
+      return config
+
+  def verify(config):
+      # Verify that configuration is valid
+      if invalid:
+          raise ConfigError("Descriptive message")
+      return True
+
+  def generate(config):
+      # Generate daemon configs
+      pass
+
+  def apply(config):
+      # Apply the generated configs to the live system
+      pass
+
+  try:
+      config = get_config()
+      verify(config)
+  except ConfigError as e:
+      print(e)
+      sys.exit(1)
+
+The **get_config()** function must convert the VyOS config to an abstract internal
+representation. No other function is allowed to call ``vyos.config.Config`` object
+methods directly. The rationale for it is that when config reads are mixed with
+other logic, it's very hard to change the config syntax since you need to weed
+out every occurence of the old syntax. If syntax-specific code is confined to a
+single function, the rest of the code can be left untouched as long as the
+internal representation remains compatible.
+
+Another advantage is testability of the code. Mocking the entire config subsystem
+is hard, while constructing an internal representation by hand is way simpler.
+
+The **verify()** function takes an internal representation of the config and checks
+if it's valid, otherwise it must raise ``VyOSError`` with an error message that
+describes the problem and possibly suggests how to fix it. It must not make any
+changes to the system. The rationale for it is again testability and, in the
+future when the config backend is ready and every script is rewritten in this
+fashion, ability to execute commit dry run ("commit test" like in JunOS) and abort
+commit before making any changes to the system if an error is found in any component.
+
+The **generate()** function generates config files for system components.
+
+The **apply()** function applies the generated configuration to the live system.
+It should use non-disruptive reload whenever possible. It may execute disruptive
+operations such as daemon process restart if a particular component does not
+support non-disruptive reload, or when the expected service degradation is minimal
+(for example, in case of auxillary services such as LLDPd). In case of high impact
+services such as VPN daemon and routing protocols, when non-disruptive reload is
+supported for some but not all types of configuration changes, scripts authors
+should make effort to determine if a configuration change can be done in a
+non-disruptive way and only resort to disruptive restart if it cannot be avoided.
+
+Unless absolutely necessary, configuration scripts should not modify the active
+configuration of system components directly. Whenever at all possible, scripts
+should generate a configuration file or files that can be applied with a single
+command such as reloading a service through systemd init. Inserting statements
+one by one is particularly discouraged, for example, when configuring netfilter
+rules, saving them to a file and loading it with iptables-restore should always
+be preferred to executing iptables directly.
+
+The **apply()** and **generate()** functions may ``raise ConfigError`` if, for
+example, the daemon failed to start with the updated config. It shouldn't be a
+substitute for proper config checking in the **verify()** function. All reasonable
+effort should be made to verify that generated configuration is valid and will
+be accepted by the daemon, including, when necessary, cross-checks with other
+VyOS configuration subtrees.
+
+Exceptions, including ``VyOSError`` (which is raised by ``vyos.config.Config`` on
+improper config operations, such as trying to use ``list_nodes()`` on a non-tag
+node) should not be silenced or caught and re-raised as config error. Sure this
+will not look pretty on user's screen, but it will make way better bug reports,
+and help users (and most VyOS users are IT professionals) do their own debugging
+as well.
+
+Coding guidelines
+-----------------
+
+Language
+********
+
+Python 3 **shall** be used. How long can we keep Python 2 alive anyway?
+
+No considerations for Python 2 compatibility **should** be taken.
+
+Formatting
+**********
+
+Tabs **shall not** be used. Every indentation level should be 4 spaces.
+
+Text generation
+***************
+
+Template processor **should** be used for generating config files. Built-in
+string formatting **may** be used for simple line-oriented formats where every
+line is self-contained, such as iptables rules. Template processor **must** be
+used for structured, multi-line formats such as those used by ISC DHCPd.
+
+The default template processor for VyOS code is jinja2.
+
+Code policy
+-----------
+
+When modifying the source code, remember these rules of the legacy elimination
+campaign:
+
+ * No new features in Perl
+ * No old style command definitions
+ * No code incompatible with Python3
+
+.. _process: https://blog.vyos.io/vyos-development-digest-10
+.. _vyos-1x: https://github.com/vyos/vyos-1x/blob/current/schema/
+.. _VyConf: https://github.com/vyos/vyconf/blob/master/data/schemata
+
-- 
cgit v1.2.3