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.

Sign up for GitHub

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

Jump to bottom

Feat/improvement for data sources #289

Open
wants to merge 132 commits into
base: fix/consistent_data_sources
Choose a base branch
from
Open

Feat/improvement for data sources #289

wants to merge 132 commits into from

Conversation

gabriel-savu
Copy link
Contributor

@gabriel-savu gabriel-savu commented Jun 30, 2022 •
edited

What does this fix or implement?

Checklist

  • PR name added as appropriate (e.g. feat:/fix:/doc:/test:/refactor:)
  • Tests added or updated
  • Documentation updated
  • Changelog updated and version incremented (label: upcoming release)
  • Github Issue linked if any
  • Jira task updated

iblindu and others added 22 commits April 27, 2022 14:39
@iblindu
fix: natgateway test (#271)
02f6e01
@iblindu
fix: k8s test (#272)
f984034 
* fix: k8s_nodepool test

* docs: updated CHANGELOG.md

* fix: k8s_nodepool test
@iblindu
fix: replaced changed images (#275)
5daf728
@iblindu
fix: bug when upgrading from a v6.0.0-beta.X version to a stable one (#…
3cdf563 
…276)
@iblindu
fix: reintroduced error for image data source when finding multiple r…
9d19695 
…esults with data source (#277)
@iblindu
docs: updated CHANGELOG.md
408c61d
@cristiGuranIonos
Feat/update dbaas sdk dep (#278)
66b5628
@iblindu
doc: updated multiple nics under the same IP Failover example, with a…
566be8c 
…n one step example (#279)
@cristiGuranIonos
Feat/increment sdk go dep (#282)
82dcd00
@cristiGuranIonos
Fix/add title to test suite (#280)
4b622ce
@iblindu
fix: crash when trying to disable autoscaling on k8s (#283)
b7fb5a7
feat: adding more lookup parameters for data sources
9199938
Merge branch 'master' of https://github.com/ionos-cloud/terraform-pro…
94e14b1 
…vider-ionoscloud into feat/improvement-for-data-sources

� Conflicts:
�	ionoscloud/data_source_image.go
�	ionoscloud/resource_natgateway_test.go
feat: improved volume lookup data source
0466c90
fix: data source user multiple parameters, only for first name
3f1ab8f
@cristiGuranIonos
feat: add nodes datasource (#285)
7faa251
@cristiGuranIonos
feat: add servers data source (#284)
c3449c6
@cristiGuranIonos
fix: servers tests (#286)
7699c0f
@cristiGuranIonos
fix: servers tests
f001848
@iblindu
feat: new product - alb (#58)
a12127f
@cristiGuranIonos
fix: daily nic import test (#288)
0dc17a7
fix: all data source lookups parameters tests
709f87b
iblindu
iblindu suggested changes Jul 1, 2022
View reviewed changes
ionoscloud/data_source_dbaas_pgsql_cluster.go Show resolved Hide resolved
ionoscloud/data_source_dbaas_pgsql_cluster.go Outdated Show resolved Hide resolved
ionoscloud/data_source_dbaas_pgsql_cluster.go Show resolved Hide resolved
ionoscloud/data_source_dbaas_pgsql_cluster.go Outdated Show resolved Hide resolved
ionoscloud/data_source_dbaas_pgsql_cluster.go Outdated Show resolved Hide resolved
ionoscloud/data_source_natgateway_rule.go Show resolved Hide resolved
ionoscloud/data_source_natgateway_rule.go Outdated Show resolved Hide resolved
ionoscloud/data_source_natgateway_rule.go Outdated Show resolved Hide resolved
ionoscloud/data_source_networkloadbalancer.go Show resolved Hide resolved
ionoscloud/data_source_networkloadbalancer.go Outdated Show resolved Hide resolved
@iblindu
Copy link
Contributor

iblindu commented Jul 1, 2022

When it comes to data-sources the general feedback is:

  • documentation should be updated with all new arguments
  • errors should also be updated where needed
  • the comment form data_source_dbaas_cluster is available in almost all data sources

This block with ListClusters should be in the else branch of partial match (like before) since if partial match is set you are making an additional unnecessary call. Also taking into consideration that you've added additional parameters, you should firstly verify nameOk, and after this the partial_match, since you don't want to filter by name if the user did not want this (if you have tests with this, something is wrong there).
On the else branch of nameOk you can make another ListClusters call to be sure that result have all clusters for the filters after.

@sonarcloud
Copy link

sonarcloud bot commented Jul 21, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

No Coverage information No Coverage information
3.3% 3.3% Duplication

Gabriel Savu and others added 30 commits December 12, 2022 20:54
fix: changed image password examples in documentation
277607d
Merge branch 'master' of https://github.com/ionos-cloud/terraform-pro…
1a7cbe4 
…vider-ionoscloud into feat/change-password-field
fix: added external provider in import alb resource
a2501ee
fix: added external provider in import resources
7485903
@adeatcu-ionos
feat: add DBaaS Mongo Template data source (#344)
3782222 
* add DBaaS Mongo Template data source

* add tests and documentation
fix: added external provider in snapshot resource, little typo in err…
a243fb1 
…or volume basic test and updated changelog
@cristiGuranIonos
fix: get nic id from entities and %w fix in printf (#346)
a5c9351
@adeatcu-ionos
fix: change the way in which we set the NIC data (#347)
2772e71
fix: added server image constant for resources tests
f371877
fix: added server image update constant for resources tests
92e424b
fix: changed image password
deb9279
@ikukschtel
First link changed (#334)
c250ade
Merge pull request #342 from ionos-cloud/feat/change-password-field
7afce7f 
feat: added random password feature
@cristiGuranIonos
Fix/update mongo version, rewrite mongo service, user state was remov…
7810d93 
…ed (#348)
fix: mix up share and edit privileges on create and removed cpu famil…
a2a63a3 
…y from cube server
@adeatcu-ionos
test: add tags for DBaaS Mongo templates (#349)
e932838
@cristiGuranIonos
fix: nodepool test (#351)
aabb99c
fix: removed cpu family from cube server docs and changelog updated
c25dc4f
Merge branch 'master' of https://github.com/ionos-cloud/terraform-pro…
7b38273 
…vider-ionoscloud into fix/create-share-mixup

# Conflicts:
#	CHANGELOG.md
Merge pull request #350 from ionos-cloud/fix/create-share-mixup
22bdc17 
fix: mix up share and edit privileges on create
@cristiGuranIonos
fix: make viable_node_pool_versions only computed (#352)
a74559a
@cristiGuranIonos
fix: server should not get its firewall_id from separate firewall res…
d93ffd8 
…ource
@cristiGuranIonos
doc: update changelog
61518b2
@cristiGuranIonos
fix: readme fix link to test suite, dbaas test use correct checking f…
7f5b0f4 
…unction (#353)
@adeatcu-ionos
fix: fix typo in group resource (#354)
e52beb4
@cristiGuranIonos
fix: allow server import with nic and firewall ids (#355)
59bb23d
@adeatcu-ionos
doc: update documentation for volumes (#356)
dc7c10d
@adeatcu-ionos
refactor: remove useless checks from services and fix typo in server …
bc949da 
…documentation (#357)
Merge branch 'master' of https://github.com/ionos-cloud/terraform-pro…
3ca9cb2 
…vider-ionoscloud into feat/improvement-for-data-sources

# Conflicts:
#	docs/data-sources/user.md
#	go.sum
#	ionoscloud/constants.go
#	ionoscloud/data_source_backup_unit.go
#	ionoscloud/data_source_datacenter.go
#	ionoscloud/data_source_dbaas_pgsql_cluster.go
#	ionoscloud/data_source_firewall.go
#	ionoscloud/data_source_group.go
#	ionoscloud/data_source_ipblock.go
#	ionoscloud/data_source_k8s_cluster.go
#	ionoscloud/data_source_k8s_node_pool.go
#	ionoscloud/data_source_lan.go
#	ionoscloud/data_source_location.go
#	ionoscloud/data_source_natgateway.go
#	ionoscloud/data_source_natgateway_rule.go
#	ionoscloud/data_source_networkloadbalancer.go
#	ionoscloud/data_source_networkloadbalancer_forwardingrule.go
#	ionoscloud/data_source_private_crossconnect.go
#	ionoscloud/data_source_snapshot.go
#	ionoscloud/data_source_user.go
#	ionoscloud/data_source_volume.go
#	ionoscloud/resource_k8s_node_pool_test.go
#	ionoscloud/resource_user_test.go
fix: merge changes from master
688d1ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cristiGuranIonos cristiGuranIonos Awaiting requested review from cristiGuranIonos

@iblindu iblindu Awaiting requested review from iblindu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@gabriel-savu @iblindu @cristiGuranIonos @avirtopeanu-ionos @gohumble @adeatcu-ionos @ikukschtel