feat!: remove deprecated features (#277)

BREAKING-CHANGE: The deprecated --no-impersonation, --no-allow-instance-profile, and --allow-instance-profile command-line arguments have been removed

Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>

src/deadline_worker_agent/api_models.py

@@ -76,9 +76,8 @@ class TaskRunAction(TypedDict):
     actionType: StepActionType
     taskId: str
     stepId: str
-    # TODO - Remove 'str' type from this; it's for an older incarnation of the API
     parameters: NotRequired[
-        dict[str, StringParameter | PathParameter | IntParameter | FloatParameter | str]
+        dict[str, StringParameter | PathParameter | IntParameter | FloatParameter]
     ]
 
 
@@ -278,12 +277,6 @@ class JobDetailsData(JobDetailsIdentifierFields):
     pathMappingRules: NotRequired[list[PathMappingRule]]
     """The path mapping rules from the service (before job attachments rules are added)"""
 
-    queueSessionRoleArn: NotRequired[str]
-    """An optional IAM role ARN corresponding used for worker sessions on the job's queue.
-    TODO Deprecated. This is the old name of the queueRoleArn field and will be deleted before
-    release.
-    """
-
     queueRoleArn: NotRequired[str]
     """An optional IAM role ARN corresponding used for worker sessions on the job's queue"""
 

src/deadline_worker_agent/boto/shim.py

@@ -167,8 +167,6 @@ def parse_sync_input_job_attachments_action(
             mapped_action = SyncInputJobAttachmentsAction(
                 sessionActionId=action_id,
                 actionType="SYNC_INPUT_JOB_ATTACHMENTS",
-                # TODO: Add this in when we support it
-                # mode=action["mode"],
             )
             if step_id := action.get("stepId", None):
                 mapped_action["stepId"] = step_id

src/deadline_worker_agent/scheduler/scheduler.py

@@ -411,7 +411,6 @@ def _sync(self, *, interruptable: bool = True) -> int:
         updated_actions, commit_completed_actions = self._updated_session_actions()
 
         #    1.2. TODO: IP address changes
-        #    1.3. TODO: metrics
 
         # 2. make request
         request: dict[str, Any] = {

src/deadline_worker_agent/sessions/job_entities/job_details.py

@@ -244,11 +244,7 @@ def from_boto(cls, job_details_data: JobDetailsData) -> JobDetails:
         )
 
         # Note: Record the empty string as a None as well.
-        queue_role_arn: str | None = (
-            job_details_data.get("queueSessionRoleArn", None)
-            or job_details_data.get("queueRoleArn", None)
-            or None
-        )
+        queue_role_arn: str | None = job_details_data.get("queueRoleArn", None)
 
         given_schema_version = TemplateSpecificationVersion(job_details_data["schemaVersion"])
 
@@ -330,8 +326,6 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData:
                             required=False,
                             fields=(
                                 Field(key="user", expected_type=str, required=True),
-                                # TODO: Remove this once the API field is removed
-                                Field(key="group", expected_type=str, required=False),
                                 Field(key="passwordArn", expected_type=str, required=True),
                             ),
                         ),
@@ -346,8 +340,6 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData:
                         Field(key="rootPrefix", expected_type=str, required=True),
                     ),
                 ),
-                # TODO - Remove queueSessionRoleArn
-                Field(key="queueSessionRoleArn", expected_type=str, required=False),
                 Field(key="queueRoleArn", expected_type=str, required=False),
             ),
         )

src/deadline_worker_agent/sessions/session.py

@@ -799,7 +799,6 @@ def exit_environment(
 
     def _notifier_callback(
         self,
-        current_action: CurrentAction,
         progress_report: ProgressReportMetadata,
     ) -> bool:
         """Callback to be passed into JobAttachments to track the file transfer.
@@ -808,22 +807,6 @@ def _notifier_callback(
         current_action is added by the Worker Agent (via partial)
         progress and status message are passed in by Job Attachments."""
         return True
-        # TODO: Since moving to the Open Job Description callback, asset sync no longer blocks
-        # the next action. Therefore we can end up in a situation where
-        # this callback attempts to re-open a completed session action
-        # and/or attempts to complete a session action in the wrong order.
-
-        # status = ActionStatus(
-        #     state=ActionState.RUNNING,
-        #     progress=float(progress_report.progress),
-        #     status_message=progress_report.progressMessage,
-        # )
-        # self._action_update_callback(SessionActionStatus(
-        #     id=current_action.definition.id,
-        #     start_time=current_action.start_time,
-        #     status=status,
-        #     update_time=datetime.now(tz=timezone.utc),
-        # ))
 
     def sync_asset_inputs(
         self,
@@ -1272,7 +1255,7 @@ def _sync_asset_outputs(
             start_time=current_action.start_time.timestamp(),
             session_dir=self._session.working_directory,
             storage_profiles_path_mapping_rules=storage_profiles_path_mapping_rules_dict,
-            on_uploading_files=partial(self._notifier_callback, current_action),
+            on_uploading_files=self._notifier_callback,
         )
 
         self.logger.info(f"Summary Statistics for file uploads:\n{upload_summary_statistics}")

src/deadline_worker_agent/startup/cli_args.py

@@ -15,17 +15,13 @@ class ParsedCommandLineArguments(Namespace):
     profile: str | None = None
     verbose: bool | None = None
     no_shutdown: bool | None = None
-    # TODO - Remove no_impersonation when removing corresponding CLI option
-    no_impersonation: bool | None = None
     run_jobs_as_agent_user: bool | None = None
     posix_job_user: str | None = None
     disallow_instance_profile: bool | None = None
     logs_dir: Path | None = None
     local_session_logs: bool | None = None
     persistence_dir: Path | None = None
     retain_session_dir: bool | None = None
-    # TODO: Remove when deprecating --no-allow-instance-profile
-    no_allow_instance_profile: bool | None = None
     host_metrics_logging: bool | None = None
     host_metrics_logging_interval_seconds: float | None = None
     structured_logs: bool | None = None
@@ -66,16 +62,6 @@ def get_argument_parser() -> ArgumentParser:
         const=True,
         default=None,
     )
-    # TODO - Remove the no-impersonation option after a deprecation period.
-    #  Remove the test named test_impersonation_mutual_exclusion at the same time
-    parser.add_argument(
-        "--no-impersonation",
-        help="(DEPRECATED: use --run-jobs-as-agent-user instead) If set, then all Jobs' session actions will run as the same user as the agent. WARNING: this is insecure - for development use only.",
-        action="store_const",
-        const=True,
-        dest="no_impersonation",
-        default=None,
-    )
     parser.add_argument(
         "--run-jobs-as-agent-user",
         help="If set, then all Jobs' session actions will run as the same user as the agent. WARNING: this is insecure - for development use only.",
@@ -112,22 +98,6 @@ def get_argument_parser() -> ArgumentParser:
         default=None,
         type=Path,
     )
-    # TODO: This is deprecated. Remove this eventually
-    parser.add_argument(
-        "--no-allow-instance-profile",
-        help="DEPRECATED. This does nothing",
-        action="store_true",
-        dest="no_allow_instance_profile",
-    )
-    # TODO: This is deprecated. Remove this eventually
-    parser.add_argument(
-        "--allow-instance-profile",
-        help="DEPRECATED. This does nothing",
-        action="store_const",
-        const=True,
-        dest="allow_instance_profile",
-        default=None,
-    )
     parser.add_argument(
         "--disallow-instance-profile",
         help="Turns on validation that the host EC2 instance profile is disassociated before starting",

src/deadline_worker_agent/startup/config.py

@@ -118,13 +118,7 @@ def __init__(
         if parsed_cli_args.no_shutdown is not None:
             settings_kwargs["no_shutdown"] = parsed_cli_args.no_shutdown
         if parsed_cli_args.run_jobs_as_agent_user is not None:
-            if parsed_cli_args.no_impersonation is not None:
-                raise ConfigurationError(
-                    "Only one of --no-impersonation or --run-jobs-as-agent-user may be supplied."
-                )
             settings_kwargs["run_jobs_as_agent_user"] = parsed_cli_args.run_jobs_as_agent_user
-        elif parsed_cli_args.no_impersonation is not None:
-            settings_kwargs["run_jobs_as_agent_user"] = parsed_cli_args.no_impersonation
         if parsed_cli_args.posix_job_user is not None:
             settings_kwargs["posix_job_user"] = parsed_cli_args.posix_job_user
         if parsed_cli_args.disallow_instance_profile is not None:

test/unit/sessions/job_entities/test_job_entities.py

@@ -216,7 +216,6 @@ def test_has_path_mapping_rules(
                     },
                     "windows": {
                         "user": "job-user",
-                        "group": "job-group",
                         "passwordArn": "job-password-arn",
                     },
                 },
@@ -268,7 +267,6 @@ def test_job_run_as_user(
                 },
                 "windows": {
                     "user": expected_user,
-                    "group": expected_group,
                     "passwordArn": expected_password_arn,
                 },
             },
@@ -314,7 +312,6 @@ def test_job_details(
                     },
                     "windows": {
                         "user": "job-user",
-                        "group": "job-group",
                         "passwordArn": "job-password-arn",
                     },
                 },

test/unit/startup/test_cli_args.py

@@ -26,11 +26,20 @@ def test_defaults(self, arg_parser: ArgumentParser) -> None:
         result = arg_parser.parse_args(args, namespace=cli_args_mod.ParsedCommandLineArguments())
 
         # THEN
+        assert result.disallow_instance_profile is None
         assert result.farm_id is None
         assert result.fleet_id is None
+        assert result.host_metrics_logging is None
+        assert result.host_metrics_logging_interval_seconds is None
+        assert result.local_session_logs is None
+        assert result.logs_dir is None
+        assert result.no_shutdown is None
+        assert result.persistence_dir is None
         assert result.profile is None
+        assert result.run_jobs_as_agent_user is None
+        assert result.retain_session_dir is None
+        assert result.structured_logs is None
         assert result.verbose is None
-        assert result.allow_instance_profile is None
 
     @pytest.mark.parametrize(
         ["farm_id"],
@@ -121,26 +130,6 @@ def test_no_shutdown(
         # THEN
         assert result.no_shutdown == expected
 
-    @pytest.mark.parametrize(
-        ("args", "expected_allow_instance_profile"),
-        (
-            pytest.param(["--allow-instance-profile"], True, id="AllowInstanceProfilePresent"),
-            pytest.param([], None, id="AllowInstanceProfileAbsent"),
-        ),
-    )
-    def test_allow_instance_profile(
-        self,
-        arg_parser: ArgumentParser,
-        args: list[str],
-        expected_allow_instance_profile: bool | None,
-    ) -> None:
-        """Asserts that the --allow-instance-profile command-line argument is parsed"""
-        # WHEN
-        result = arg_parser.parse_args(args, namespace=cli_args_mod.ParsedCommandLineArguments())
-
-        # THEN
-        assert result.allow_instance_profile == expected_allow_instance_profile
-
     @pytest.mark.parametrize(
         ("args", "expected_cleanup_session_user_processes"),
         (

test/unit/startup/test_config.py

@@ -386,17 +386,6 @@ def test_uses_retain_session_dir(
         # THEN
         assert config.retain_session_dir == retain_session_dir
 
-    def test_impersonation_mutual_exclusion(
-        self, parsed_args: config_mod.ParsedCommandLineArguments
-    ):
-        # GIVEN
-        parsed_args.no_impersonation = True
-        parsed_args.run_jobs_as_agent_user = True
-
-        # THEN
-        with pytest.raises(config_mod.ConfigurationError):
-            config_mod.Configuration(parsed_cli_args=parsed_args)
-
 
 class TestInit:
     """Tests for Configuration.__init__"""