Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support realized_id in ip pool data source #1008

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Conversation

annakhm
Copy link
Collaborator

@annakhm annakhm commented Oct 20, 2023

This id should be used in transport node resource
Signed-off-by: Anna Khmelnitsky akhmelnitsky@vmware.com

@annakhm annakhm requested a review from ksamoray October 20, 2023 23:34
@annakhm
Copy link
Collaborator Author

annakhm commented Oct 20, 2023

/test-all

@ksamoray
Copy link
Collaborator

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

@annakhm
Copy link
Collaborator Author

annakhm commented Oct 23, 2023

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

It is just more convenient.. we expose this attribute in resource, makes sense to do this in data source as well

@ksamoray
Copy link
Collaborator

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

It is just more convenient.. we expose this attribute in resource, makes sense to do this in data source as well

But policy_ip_pool resource doesn't have this attribute.

@salv-orlando
Copy link
Member

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

It is just more convenient.. we expose this attribute in resource, makes sense to do this in data source as well

But policy_ip_pool resource doesn't have this attribute.

It's always good to be consistent indeed. Also, as the attribute in API spec is called realization_id, I would stay consistent also with naming

@ksamoray
Copy link
Collaborator

ksamoray commented Nov 5, 2023

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

It is just more convenient.. we expose this attribute in resource, makes sense to do this in data source as well

But policy_ip_pool resource doesn't have this attribute.

It's always good to be consistent indeed. Also, as the attribute in API spec is called realization_id, I would stay consistent also with naming

We already have realized_id attributes in other objects. It's better to remain consistent within the provider than being consistent with NSX.

@annakhm
Copy link
Collaborator Author

annakhm commented Nov 6, 2023

nsxt_policy_realization_info data source is usable with IP pools, why do we need this specifically for IP pool?

It is just more convenient.. we expose this attribute in resource, makes sense to do this in data source as well

But policy_ip_pool resource doesn't have this attribute.

It's always good to be consistent indeed. Also, as the attribute in API spec is called realization_id, I would stay consistent also with naming

We already have realized_id attributes in other objects. It's better to remain consistent within the provider than being consistent with NSX.

I've added this attribute to the resource as well

@annakhm
Copy link
Collaborator Author

annakhm commented Nov 6, 2023

/test-all

1 similar comment
@ksamoray
Copy link
Collaborator

ksamoray commented Nov 7, 2023

/test-all

Copy link
Collaborator

@ksamoray ksamoray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please rebase on top of master so tests will pass.
I think that maybe in a couple of more we can justify a getRealizedIdSchema() func :)

This id should be used in transport node resource
Signed-off-by: Anna Khmelnitsky <akhmelnitsky@vmware.com>
@annakhm
Copy link
Collaborator Author

annakhm commented Nov 7, 2023

/test-all

@annakhm annakhm merged commit d7b914e into master Nov 7, 2023
5 checks passed
@annakhm annakhm linked an issue Nov 7, 2023 that may be closed by this pull request
@annakhm annakhm deleted the pool-realized-id branch August 2, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

data source nsxt_policy_ip_pool not retrieving the correct id
3 participants