reconcile CLI/config priority

- avoid CLI defaults clobbering values from config.
  Mostly by using `default=None`, and adding `if args.name is not None` checks
  before assigning attributes

repo2docker/__main__.py

@@ -93,7 +93,7 @@ def get_argparser():
 
     argparser.add_argument(
         "--json-logs",
-        default=False,
+        default=None,
         action="store_true",
         help="Emit JSON logs instead of human readable logs",
     )
@@ -108,7 +108,7 @@ def get_argparser():
 
     argparser.add_argument(
         "--image-name",
-        help=("Name of image to be built. If unspecified will be " "autogenerated"),
+        help="Name of image to be built. If unspecified will be autogenerated",
         type=validate_image_name,
     )
 
@@ -126,9 +126,7 @@ def get_argparser():
         "--no-build",
         dest="build",
         action="store_false",
-        help=(
-            "Do not actually build the image. Useful in conjunction " "with --debug."
-        ),
+        help="Do not actually build the image. Useful in conjunction with --debug.",
     )
 
     argparser.add_argument(
@@ -164,6 +162,7 @@ def get_argparser():
         "--publish-all",
         "-P",
         dest="all_ports",
+        default=None,
         action="store_true",
         help="Publish all exposed ports to random host ports.",
     )
@@ -171,6 +170,7 @@ def get_argparser():
     argparser.add_argument(
         "--no-clean",
         dest="clean",
+        default=None,
         action="store_false",
         help="Don't clean up remote checkouts after we are done",
     )
@@ -250,6 +250,7 @@ def get_argparser():
     return argparser
 
 
+# Note: only used by sphinx-autoprogram
 argparser = get_argparser()
 
 
@@ -274,12 +275,18 @@ def make_r2d(argv=None):
     args, traitlet_args = argparser.parse_known_args(argv)
 
     r2d = Repo2Docker()
-    r2d.parse_command_line(traitlet_args)
 
     if args.debug:
         r2d.log_level = logging.DEBUG
 
+    # load CLI after config file, for correct priority
     r2d.load_config_file(args.config)
+    r2d.parse_command_line(traitlet_args)
+
+    if args.debug:
+        # re-apply debug in case log_level was also set via config
+        r2d.log_level = logging.DEBUG
+
     if args.appendix:
         r2d.appendix = args.appendix
 
@@ -317,7 +324,8 @@ def make_r2d(argv=None):
         # we will pick a name after fetching the repository
         r2d.output_image_spec = ""
 
-    r2d.json_logs = args.json_logs
+    if args.json_logs is not None:
+        r2d.json_logs = args.json_logs
 
     r2d.dry_run = not args.build
 
@@ -341,29 +349,33 @@ def make_r2d(argv=None):
         src, dest = v.split(":")
         r2d.volumes[src] = dest
 
-    r2d.run_cmd = args.cmd
+    if args.cmd:
+        r2d.run_cmd = args.cmd
 
     if args.all_ports and not r2d.run:
-        print(
-            "To publish user defined port mappings, the container must " "also be run"
-        )
+        print("To publish user-defined port mappings, the container must also be run")
         sys.exit(1)
 
     if args.ports and not r2d.run:
-        print(
-            "To publish user defined port mappings, the container must " "also be run"
-        )
+        print("To publish user-defined port mappings, the container must also be run")
         sys.exit(1)
 
     if args.ports and len(args.ports) > 1 and not r2d.run_cmd:
         print(
-            "To publish user defined port mapping, user must specify "
-            "the command to run in the container"
+            "To publish user-defined port mapping, "
+            "you must specify the command to run in the container"
         )
         sys.exit(1)
 
-    r2d.ports = validate_and_generate_port_mapping(args.ports)
-    r2d.all_ports = args.all_ports
+    if args.ports:
+        # override or update, if also defined in config file?
+        if r2d.ports:
+            r2d.log.warning(
+                f"Ignoring port configuration from config (ports={r2d.ports}), overridden by CLI"
+            )
+        r2d.ports = validate_and_generate_port_mapping(args.ports)
+    if args.all_ports is not None:
+        r2d.all_ports = args.all_ports
 
     if args.user_id:
         r2d.user_id = args.user_id
@@ -401,13 +413,15 @@ def make_r2d(argv=None):
     if args.engine:
         r2d.engine = args.engine
 
-    r2d.environment = args.environment
+    if args.environment:
+        # extend any environment config from a config file
+        r2d.environment.extend(args.environment)
 
     # if the source exists locally we don't want to delete it at the end
     # FIXME: Find a better way to figure out if repo is 'local'. Push this into ContentProvider?
     if os.path.exists(args.repo):
         r2d.cleanup_checkout = False
-    else:
+    elif args.clean is not None:
         r2d.cleanup_checkout = args.clean
 
     if args.target_repo_dir:

repo2docker/app.py

@@ -332,7 +332,7 @@ class Repo2Docker(Application):
     )
 
     run = Bool(
-        False,
+        True,
         help="""
         Run docker image after building
         """,