You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "vandonr-amz (via GitHub)" <gi...@apache.org> on 2023/03/03 23:28:03 UTC

[GitHub] [airflow] vandonr-amz opened a new pull request, #29911: custom waiters with dynamic values, applied to appflow

vandonr-amz opened a new pull request, #29911:
URL: https://github.com/apache/airflow/pull/29911

    - add a mechanism to allow custom waiters to take dynamic values at runtime by editing the waiter config on the fly
       - placeholders for dynamic values take the form `<CAPS_OR_123>` in the waiter config's json
       - they are meant to be used in the `argument` field, which describes what's checked, but there is no technical limitation to use them elsewhere (for instance to specify the `expected` value at runtime)
       - the waiter config still needs to be valid json with those placeholders unexpanded
       - there is some validation, checking that necessary parameters have been provided (WARN level logging if not, or if useless parameters are provided)
   
    - apply this new capability in transforming appflow `wait_for_completion` code to use a waiter (follow-up on #28869)
       - not using the execution ID in this waiter would mean making unchecked assumptions on the data we're reading.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134484142


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   ...but then as we handle the configuration for all the waiters at once, it'd fail/warn if params for other waiters are missing, which is bad. We'd have to systematically reconstruct a json for only the waiter we're going to use.
   
   It'd be:
   
   - json parse
   - remove all waiters we're not interested in from ["waiters"]
   - convert to string again, do jinja magic on string
   - parse to json again



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] Taragolis commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1128428786


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   Just a question, why not to use Jinja for render templates instead of create own template mechanism?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134386280


##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -794,25 +794,66 @@ def waiter_path(self) -> PathLike[str] | None:
         path = Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
         return path if path.exists() else None
 
-    def get_waiter(self, waiter_name: str) -> Waiter:
+    def get_waiter(self, waiter_name: str, parameters: dict[str, str] | None = None) -> Waiter:
         """
         First checks if there is a custom waiter with the provided waiter_name and
         uses that if it exists, otherwise it will check the service client for a
         waiter that matches the name and pass that through.
 
         :param waiter_name: The name of the waiter.  The name should exactly match the
             name of the key in the waiter model file (typically this is CamelCase).
+        :param parameters: will scan the waiter config for the keys of that dict, and replace them with the
+            corresponding value. If a custom waiter has such keys to be expanded, they need to be provided
+            here.
         """
         if self.waiter_path and (waiter_name in self._list_custom_waiters()):
             # Technically if waiter_name is in custom_waiters then self.waiter_path must
             # exist but MyPy doesn't like the fact that self.waiter_path could be None.
             with open(self.waiter_path) as config_file:
-                config = json.load(config_file)
-                return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
+                config_text = config_file.read()
+
+            if parameters:  # expand some variables in the config if needed
+                config_text = self._apply_parameters_value(config_text, parameters)
+            config = json.loads(config_text)
+            self._check_remaining_params(config, waiter_name)
+            return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
         # If there is no custom waiter found for the provided name,
         # then try checking the service's official waiters.
         return self.conn.get_waiter(waiter_name)
 
+    waiter_config_key_name_format = re.compile("^[A-Z0-9_]+$")
+    waiter_config_key_placeholder_format = re.compile("<[A-Z0-9_]+>")

Review Comment:
   I wanted to have the variable close to the usage so that when reading the function you can just raise your eyes to see the variable. And static checks are not complaining, so I guess all good ?



##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -794,25 +794,66 @@ def waiter_path(self) -> PathLike[str] | None:
         path = Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
         return path if path.exists() else None
 
-    def get_waiter(self, waiter_name: str) -> Waiter:
+    def get_waiter(self, waiter_name: str, parameters: dict[str, str] | None = None) -> Waiter:
         """
         First checks if there is a custom waiter with the provided waiter_name and
         uses that if it exists, otherwise it will check the service client for a
         waiter that matches the name and pass that through.
 
         :param waiter_name: The name of the waiter.  The name should exactly match the
             name of the key in the waiter model file (typically this is CamelCase).
+        :param parameters: will scan the waiter config for the keys of that dict, and replace them with the
+            corresponding value. If a custom waiter has such keys to be expanded, they need to be provided
+            here.
         """
         if self.waiter_path and (waiter_name in self._list_custom_waiters()):
             # Technically if waiter_name is in custom_waiters then self.waiter_path must
             # exist but MyPy doesn't like the fact that self.waiter_path could be None.
             with open(self.waiter_path) as config_file:
-                config = json.load(config_file)
-                return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
+                config_text = config_file.read()
+
+            if parameters:  # expand some variables in the config if needed
+                config_text = self._apply_parameters_value(config_text, parameters)
+            config = json.loads(config_text)
+            self._check_remaining_params(config, waiter_name)
+            return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
         # If there is no custom waiter found for the provided name,
         # then try checking the service's official waiters.
         return self.conn.get_waiter(waiter_name)
 
+    waiter_config_key_name_format = re.compile("^[A-Z0-9_]+$")
+    waiter_config_key_placeholder_format = re.compile("<[A-Z0-9_]+>")
+
+    def _apply_parameters_value(self, config: str, parameters: dict[str, str]) -> str:
+        """replaces the given parameters in the config with their value"""
+        for k, v in parameters.items():
+            if not self.waiter_config_key_name_format.match(k):
+                raise AirflowException(
+                    f"{k} is not an accepted key for waiter configuration templatization. "
+                    "Use only capitals, digits and underscores."
+                )
+            if config.find(k) == -1:

Review Comment:
   yes good catch



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134641064


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   ok I found a better solution, where I'm targeting only the acceptors' argument with jinja, so that I can restrict it to the waiter wanted without doing to much manipulations



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134400061


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   hmm that could be interesting yeah, I'll see how it could happen



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] ferruzzi commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134756138


##########
tests/providers/amazon/aws/waiters/test.json:
##########
@@ -0,0 +1,44 @@
+{
+    "version": 2,
+    "waiters": {
+        "wait_for_test": {
+            "operation": "GetEnvironment",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": true,
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "'{{PARAM_1}}' == '{{PARAM_2}}'"
+                }
+            ]
+        },
+        "other_wait": {
+            "operation": "GetEnvironment",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "blah",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "blah"
+                }
+            ]
+        },
+        "bad_param_wait": {
+            "operation": "GetEnvironment",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "blah",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "{{not a valid jinja template 💀}}"

Review Comment:
   LOLOLOL :skull:  :heart: 



##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -794,25 +794,46 @@ def waiter_path(self) -> PathLike[str] | None:
         path = Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
         return path if path.exists() else None
 
-    def get_waiter(self, waiter_name: str) -> Waiter:
+    def get_waiter(self, waiter_name: str, parameters: dict[str, str] | None = None) -> Waiter:
         """
         First checks if there is a custom waiter with the provided waiter_name and
         uses that if it exists, otherwise it will check the service client for a
         waiter that matches the name and pass that through.
 
         :param waiter_name: The name of the waiter.  The name should exactly match the
             name of the key in the waiter model file (typically this is CamelCase).
+        :param parameters: will scan the waiter config for the keys of that dict, and replace them with the
+            corresponding value. If a custom waiter has such keys to be expanded, they need to be provided
+            here.
         """
         if self.waiter_path and (waiter_name in self._list_custom_waiters()):
             # Technically if waiter_name is in custom_waiters then self.waiter_path must
             # exist but MyPy doesn't like the fact that self.waiter_path could be None.
             with open(self.waiter_path) as config_file:
-                config = json.load(config_file)
-                return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
+                config = json.loads(config_file.read())
+
+            config = self._apply_parameters_value(config, waiter_name, parameters)
+            return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
         # If there is no custom waiter found for the provided name,
         # then try checking the service's official waiters.
         return self.conn.get_waiter(waiter_name)
 
+    @staticmethod
+    def _apply_parameters_value(config: dict, waiter_name: str, parameters: dict[str, str] | None) -> dict:
+        """Replaces potential jinja templates in acceptors definition"""
+        # only process the waiter we're going to use to not raise errors for missing params for other waiters.
+        acceptors = config["waiters"][waiter_name]["acceptors"]
+        for a in acceptors:
+            arg = a["argument"]
+            template = jinja2.Template(arg, autoescape=False, undefined=jinja2.StrictUndefined)
+            try:
+                a["argument"] = template.render(parameters or {})
+            except jinja2.UndefinedError as e:
+                raise AirflowException(
+                    f"Parameter was not supplied for templated waiter's acceptor '{arg}'", e
+                )
+        return config
+

Review Comment:
   Yeah.  I think I like this solution.  I could nitpick the variable names but I always do that. :P 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] ferruzzi commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134751159


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   That sounds promising :+1: 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk merged pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29911:
URL: https://github.com/apache/airflow/pull/29911


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] o-nikolas commented on pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #29911:
URL: https://github.com/apache/airflow/pull/29911#issuecomment-1477316589

   @vincbeck and @Taragolis are you happy with the changes in response to your comments?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vincbeck commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1126630419


##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -794,25 +794,66 @@ def waiter_path(self) -> PathLike[str] | None:
         path = Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
         return path if path.exists() else None
 
-    def get_waiter(self, waiter_name: str) -> Waiter:
+    def get_waiter(self, waiter_name: str, parameters: dict[str, str] | None = None) -> Waiter:
         """
         First checks if there is a custom waiter with the provided waiter_name and
         uses that if it exists, otherwise it will check the service client for a
         waiter that matches the name and pass that through.
 
         :param waiter_name: The name of the waiter.  The name should exactly match the
             name of the key in the waiter model file (typically this is CamelCase).
+        :param parameters: will scan the waiter config for the keys of that dict, and replace them with the
+            corresponding value. If a custom waiter has such keys to be expanded, they need to be provided
+            here.
         """
         if self.waiter_path and (waiter_name in self._list_custom_waiters()):
             # Technically if waiter_name is in custom_waiters then self.waiter_path must
             # exist but MyPy doesn't like the fact that self.waiter_path could be None.
             with open(self.waiter_path) as config_file:
-                config = json.load(config_file)
-                return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
+                config_text = config_file.read()
+
+            if parameters:  # expand some variables in the config if needed
+                config_text = self._apply_parameters_value(config_text, parameters)
+            config = json.loads(config_text)
+            self._check_remaining_params(config, waiter_name)
+            return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
         # If there is no custom waiter found for the provided name,
         # then try checking the service's official waiters.
         return self.conn.get_waiter(waiter_name)
 
+    waiter_config_key_name_format = re.compile("^[A-Z0-9_]+$")
+    waiter_config_key_placeholder_format = re.compile("<[A-Z0-9_]+>")
+
+    def _apply_parameters_value(self, config: str, parameters: dict[str, str]) -> str:
+        """replaces the given parameters in the config with their value"""
+        for k, v in parameters.items():
+            if not self.waiter_config_key_name_format.match(k):
+                raise AirflowException(
+                    f"{k} is not an accepted key for waiter configuration templatization. "
+                    "Use only capitals, digits and underscores."
+                )
+            if config.find(k) == -1:

Review Comment:
   We should try to find `f"<{k}>"` instead of `k`. There is a hole here, if a user forget to surround the variable with `<>`, it will not fail here but at line `841` while doing the replace



##########
airflow/providers/amazon/aws/hooks/base_aws.py:
##########
@@ -794,25 +794,66 @@ def waiter_path(self) -> PathLike[str] | None:
         path = Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
         return path if path.exists() else None
 
-    def get_waiter(self, waiter_name: str) -> Waiter:
+    def get_waiter(self, waiter_name: str, parameters: dict[str, str] | None = None) -> Waiter:
         """
         First checks if there is a custom waiter with the provided waiter_name and
         uses that if it exists, otherwise it will check the service client for a
         waiter that matches the name and pass that through.
 
         :param waiter_name: The name of the waiter.  The name should exactly match the
             name of the key in the waiter model file (typically this is CamelCase).
+        :param parameters: will scan the waiter config for the keys of that dict, and replace them with the
+            corresponding value. If a custom waiter has such keys to be expanded, they need to be provided
+            here.
         """
         if self.waiter_path and (waiter_name in self._list_custom_waiters()):
             # Technically if waiter_name is in custom_waiters then self.waiter_path must
             # exist but MyPy doesn't like the fact that self.waiter_path could be None.
             with open(self.waiter_path) as config_file:
-                config = json.load(config_file)
-                return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
+                config_text = config_file.read()
+
+            if parameters:  # expand some variables in the config if needed
+                config_text = self._apply_parameters_value(config_text, parameters)
+            config = json.loads(config_text)
+            self._check_remaining_params(config, waiter_name)
+            return BaseBotoWaiter(client=self.conn, model_config=config).waiter(waiter_name)
         # If there is no custom waiter found for the provided name,
         # then try checking the service's official waiters.
         return self.conn.get_waiter(waiter_name)
 
+    waiter_config_key_name_format = re.compile("^[A-Z0-9_]+$")
+    waiter_config_key_placeholder_format = re.compile("<[A-Z0-9_]+>")

Review Comment:
   nit. In terms of convention, we do want to put them here or at the top with the other instance variables?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134438519


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   I can do something relatively simple like
   
       template = Template(config)
       return template.render(parameters)
   
   but then we lose all the checks on whether variables are provided or not, or if too many of them were present.
   
   Just tried it, and if we forget to pass the variable for the template, jinja replaces them with an empty string, which can lead to surprising silent behavior I think. Like, if we use it to pass the name of a job that should appear in a list for instance (as I plan to do), we might wait forever for a job with empty name, and it could be quite hard to debug.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] ferruzzi commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134512237


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   I haven't really played with Jinja but that sounds like an interesting option.  
   
   ```
       json parse
       remove all waiters we're not interested in from ["waiters"]
       convert to string again, do jinja magic on string
       parse to json again
   ```
   
   That sounds very inefficient :( 
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] vandonr-amz commented on a diff in pull request #29911: custom waiters with dynamic values, applied to appflow

Posted by "vandonr-amz (via GitHub)" <gi...@apache.org>.
vandonr-amz commented on code in PR #29911:
URL: https://github.com/apache/airflow/pull/29911#discussion_r1134448019


##########
airflow/providers/amazon/aws/waiters/appflow.json:
##########
@@ -0,0 +1,30 @@
+{
+    "version": 2,
+    "waiters": {
+        "run_complete": {
+            "operation": "DescribeFlowExecutionRecords",
+            "delay": 15,
+            "maxAttempts": 60,
+            "acceptors": [
+                {
+                    "expected": "Successful",
+                    "matcher": "path",
+                    "state": "success",
+                    "argument": "flowExecutions[?executionId=='<EXECUTION_ID>'].executionStatus"

Review Comment:
   looks like we can get an error from jinja in that case by specifying `undefined=StrictUndefined` in the template



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org