Make Subscription Source Type Optional (#1107)

* make subscription source type optional

* fix failing tests

* update more tests

* lint

* update var_type type hint

* update docs and standardize none checks

Author: asonnenschein

docs/cli/cli-subscriptions.md

@@ -21,7 +21,6 @@ Since there is no UI to easily create subscriptions we’ll start with making a
 {
   "name": "First Subscription",
   "source": {
-    "type": "catalog",
     "parameters": {
       "asset_types": [
         "ortho_analytic_8b"
@@ -421,7 +420,6 @@ for details. To constrain data delivery by space and time, you will use the
 
 ```sh
 planet subscriptions request-pv \
-    --var-type biomass_proxy \
     --var-id BIOMASS-PROXY_V3.0_10 \
     --geometry geometry.geojson \
     --start-time 2022-08-24T00:00:00-07:00 > request-pv.json

planet/cli/subscriptions.py

@@ -483,7 +483,7 @@ def request_catalog(item_types,
 @translate_exceptions
 @click.option(
     '--var-type',
-    required=True,
+    required=False,
     help='A Planetary Variable type. See documentation for all available types.'
 )
 @click.option(

planet/subscription_request.py

@@ -281,11 +281,11 @@ def catalog_source(
     if time_range_type:
         parameters['time_range_type'] = time_range_type
 
-    return {"type": "catalog", "parameters": parameters}
+    return {"parameters": parameters}
 
 
 def planetary_variable_source(
-    var_type: str,
+    var_type: Optional[str],
     var_id: str,
     geometry: Union[dict, str],
     start_time: datetime,
@@ -305,7 +305,8 @@ def planetary_variable_source(
 
     Parameters:
         var_type: Planetary Variable type. See documentation for all
-            available types.
+            available types.  Used to be a required parameter but
+            is now optional and can be 'None'.
         var_id: A Planetary Variable ID. See documenation for all
             available IDs.
         geometry: The area of interest of the subscription that will be
@@ -369,7 +370,10 @@ def planetary_variable_source(
         except AttributeError:
             raise ClientError('Could not convert end_time to an iso string')
 
-    return {"type": var_type, "parameters": parameters}
+    source: dict[str, Any] = {"parameters": parameters}
+    if var_type:
+        source["type"] = var_type
+    return source
 
 
 def _datetime_to_rfc3339(value: datetime) -> str:

tests/integration/test_subscriptions_cli.py

@@ -306,7 +306,6 @@ def test_subscriptions_results_success(invoke, options, expected_count):
 def test_request_base_success(invoke, geom_geojson):
     """Request command succeeds."""
     source = json.dumps({
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -344,7 +343,6 @@ def test_request_base_clip_to_source(geom_fixture, request, invoke):
     """Clip to source using command line option."""
     geom = request.getfixturevalue(geom_fixture)
     source = json.dumps({
-        "type": "catalog",
         "parameters": {
             "geometry": geom,
             "start_time": "2021-03-01T00:00:00Z",
@@ -370,7 +368,6 @@ def test_request_base_clip_to_source(geom_fixture, request, invoke):
 def test_request_catalog_success(mock_bundles, invoke, geom_geojson):
     """Request-catalog command succeeds"""
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -398,24 +395,33 @@ def test_subscriptions_results_csv(invoke):
     assert result.output.splitlines() == ["id,status", "1234-abcd,SUCCESS"]
 
 
-@pytest.mark.parametrize(
-    "geom", ["geom_geojson", "geom_reference", "str_geom_reference"])
-def test_request_pv_success(invoke, geom, request):
+@pytest.mark.parametrize("geom, source_type",
+                         [("geom_geojson", "biomass_proxy"),
+                          ("geom_reference", None),
+                          ("str_geom_reference", None)])
+def test_request_pv_success(invoke, geom, source_type, request):
     """Request-pv command succeeds"""
     geom = request.getfixturevalue(geom)
     if isinstance(geom, dict):
         geom = json.dumps(geom)
-    result = invoke([
+    cmd = [
         "request-pv",
-        "--var-type=biomass_proxy",
         "--var-id=BIOMASS-PROXY_V3.0_10",
         f"--geometry={geom}",
         "--start-time=2021-03-01T00:00:00",
-    ])
+    ]
+
+    if source_type:
+        cmd.append(f"--var-type={source_type}")
+
+    result = invoke(cmd)
 
     assert result.exit_code == 0  # success.
     source = json.loads(result.output)
-    assert source["type"] == "biomass_proxy"
+    if source_type:
+        assert source["type"] == "biomass_proxy"
+    else:
+        assert "type" not in source
     assert source["parameters"]["id"] == "BIOMASS-PROXY_V3.0_10"
 
 
@@ -488,7 +494,6 @@ def test_request_hosting(invoke,
                          expected_success):
     """Test request command with various hosting and collection ID options."""
     source = json.dumps({
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",

tests/unit/test_subscription_request.py

@@ -24,7 +24,6 @@
 
 def test_build_request_success(geom_geojson):
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -69,7 +68,6 @@ def test_build_request_success(geom_geojson):
 def test_build_request_clip_to_source_success(geom_geojson):
     """Without a clip tool we can clip to source."""
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -95,7 +93,6 @@ def test_build_request_clip_to_source_success(geom_geojson):
 def test_build_request_clip_to_source_failure(geom_geojson):
     """With a clip tool we can not clip to source."""
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -121,7 +118,6 @@ def test_build_request_clip_to_source_failure(geom_geojson):
 
 def test_build_request_host_sentinel_hub_no_collection(geom_geojson):
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -145,7 +141,6 @@ def test_build_request_host_sentinel_hub_no_collection(geom_geojson):
 
 def test_build_request_host_sentinel_hub_with_collection(geom_geojson):
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -174,7 +169,6 @@ def test_build_request_host_sentinel_hub_with_collection(geom_geojson):
 
 def test_build_request_host_sentinel_hub_create_configuration(geom_geojson):
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -203,7 +197,6 @@ def test_build_request_host_sentinel_hub_create_configuration(geom_geojson):
 def test_build_request_host_sentinel_hub_collection_configuration(
         geom_geojson):
     source = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -242,7 +235,6 @@ def test_catalog_source_success(geom_geojson, mock_bundles):
     )
 
     expected = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -270,7 +262,6 @@ def test_catalog_source_featurecollection(featurecollection_geojson,
     )
 
     expected = {
-        "type": "catalog",
         "parameters": {
             "geometry": geom_geojson,
             "start_time": "2021-03-01T00:00:00Z",
@@ -558,6 +549,7 @@ def test_toar_tool_success():
     [
         ("biomass_proxy", "BIOMASS-PROXY_V3.0_10"),  # actual real type and id.
         ("var1", "VAR1-ABCD"),  # nonsense type and id
+        (None, "BIOMASS-PROXY_V3.0_10"),  # None type with valid id
     ])
 def test_pv_source_success(geom_geojson, var_type, var_id):
     """Configure a planetary variable subscription source."""
@@ -569,7 +561,10 @@ def test_pv_source_success(geom_geojson, var_type, var_id):
         end_time=datetime(2021, 3, 2),
     )
 
-    assert source["type"] == var_type
+    if var_type:
+        assert source["type"] == var_type
+    else:
+        assert "type" not in source
     params = source["parameters"]
     assert params["id"] == var_id
     assert params["geometry"] == geom_geojson