DynamoDBHook - waiter_path() to consider resource_type or client_type

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging

[utkarsharma2]

Currently, when adding a custom waiter on DynamoDBHook, it fails to register the custom waiter because of the following logic:

airflow/airflow/providers/amazon/aws/hooks/base_aws.py

Lines 795 to 798 in 2ce1130

	@cached_property
	defwaiter_path(self) ->PathLike[str] |None:
	path=Path(__file__).parents[1].joinpath(f"waiters/{self.client_type}.json").resolve()
	returnpathifpath.exists() elseNone

It requires the self.client_type to be set, if self.client_type is not set we should pick self.resource_type instead.

closes: #30613

[utkarsharma2]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

[o-nikolas]

@o-nikolas@eladkal can you please confirm, if it's okay to use resource_type if client_type is missing?

If you've tested it and it's working then it's likely perfectly fine.

CCing @ferruzzi as well, he's done a lot of the work on our latest waiter code.

[ferruzzi]

What Niko said :P I added that bit that you are modifying. I know of no reason why it wouldn't work. If you've tested it and it works, I'll back the change. So far DynamoDB is one of the only services I've run into that seems to prefer Resources over Clients. I'm sure there are others, I just haven't really used them. If it works as a Resource, it sounds like a good change to me. Glad to see others looking at making use of the custom waiters, I really like how they simplify things for us.

[utkarsharma2]

@o-nikolas@ferruzzi Yes, I tested it locally and added relevant info to the ticket. I also added a test case to validate. PTAL.

@ferruzzi Yup, good work on waiters. This seems like a neat way to standardize the polling process. :)

[ferruzzi]

LGTM

[o-nikolas]

Can you assert the path is what you expect rather than just not falsey? That will ensure your code to get the right filename is working as you expect and won't regress due to future code changes..

[utkarsharma2]

@o-nikolas Asserted the expected path

[potiuk]

Random flake (we need to fix it soon). MErging