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

New, very simple data sources for GCR Repo and Image. #954

Merged
merged 9 commits into from
Jan 18, 2018

Conversation

nat-henderson
Copy link
Contributor

I think this fixed #607, but I'm a little fuzzy on the use-case there, so I'd like someone who requested / thumbs-up'd that to look at the code and make sure it's helpful before I invest the time in writing the docs.

@mahmoudian1
Copy link

Thanks @ndmckinley looks good

@@ -75,7 +76,8 @@ func Provider() terraform.ResourceProvider {
"google_compute_region_instance_group": dataSourceGoogleComputeRegionInstanceGroup(),
"google_container_cluster": dataSourceGoogleContainerCluster(),
"google_container_engine_versions": dataSourceGoogleContainerEngineVersions(),
"google_active_folder": dataSourceGoogleActiveFolder(),
"google_gcr_repository": dataSourceGoogleGcrRepo(),
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use the acronym GCR or expand it to container_registry.

It is also weird to have google_g(oogle)c(ontainer)r(egistry)_repository. google is repeated twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed - expanded it.

@@ -9,7 +9,7 @@ import (
func TestDataSourceGoogleGcrRepository(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename the tests too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}

func gcrRepoRead(d *schema.ResourceData, meta interface{}) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rosbo rosbo self-assigned this Jan 18, 2018
@nat-henderson
Copy link
Contributor Author

For reasons I can't figure out, these new docs are crashing the website. I know it's got to be these because it doesn't happen on master, and it does happen on this branch, even with the branch rebased on top of master. The error message is not helping me:

NoMethodError: undefined method `force_encoding' for nil:NilClass              
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:289:in `ensure in binary'                                                                      
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:289:in `binary' 
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:279:in `extract_magic_comment'                                                                 
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:275:in `extract_encoding'                                                                      
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:191:in `precompiled'                                                                           
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/erb.rb:57:in `precompiled'  
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/erb.rb:104:in `precompiled' 
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:245:in `compile_template_method'                                                               
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:240:in `compiled_method'                                                                       
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:169:in `evaluate'                                                                              
        /usr/local/bundle/gems/tilt-1.4.1/lib/tilt/template.rb:103:in `render' 
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/rendering.rb:313:in `render_individual_file'                           
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/rendering.rb:153:in `render_template'                                  
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/sitemap/resource.rb:127:in `block in render'                                           
        /usr/local/bundle/gems/activesupport-4.2.8/lib/active_support/notifications.rb:166:in `instrument'                                                    
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/util.rb:41:in `instrument'                                                             
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/application.rb:244:in `instrument'                                                     
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/sitemap/resource.rb:14:in `instrument'                                                 
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/sitemap/resource.rb:100:in `render'                                                    
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/request.rb:260:in `process_request'                                    
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/request.rb:210:in `block in call!'                                     
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/request.rb:209:in `catch'                                              
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/request.rb:209:in `call!'                                              
        /usr/local/bundle/gems/middleman-core-3.4.1/lib/middleman-core/core_extensions/request.rb:195:in `call'                                               
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/urlmap.rb:66:in `block in call'                                                                            
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/urlmap.rb:50:in `each'      
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/urlmap.rb:50:in `call'      
        /usr/local/bundle/gems/rack-livereload-0.3.16/lib/rack/livereload.rb:23:in `_call'                                                                    
        /usr/local/bundle/gems/rack-livereload-0.3.16/lib/rack/livereload.rb:14:in `call'                                                                     
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/showexceptions.rb:24:in `call'                                                                             
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/head.rb:13:in `call'        
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/lint.rb:49:in `_call'       
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/lint.rb:37:in `call'        
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/builder.rb:153:in `call'    
        /usr/local/bundle/gems/rack-1.6.8/lib/rack/handler/webrick.rb:88:in `service'                                                                         
        /usr/local/lib/ruby/2.3.0/webrick/httpserver.rb:140:in `service'       
        /usr/local/lib/ruby/2.3.0/webrick/httpserver.rb:96:in `run'            
        /usr/local/lib/ruby/2.3.0/webrick/server.rb:296:in `block in start_thread'

Any ideas?

@rosbo
Copy link
Contributor

rosbo commented Jan 18, 2018

Rename also the go files.


This data source fetches the project name, and provides the appropriate URLs to use for container registry for this project.

The URLs are computed entirely offline - as long as the project exists, they will be valid, but this data source does not contact GCR at any point.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to use the GCR acronym, you should spell it out fully the first time you use it and add the acronym between parentheses after it.

"github.com/hashicorp/terraform/helper/schema"
)

func dataSourceGoogleContainerRepo() *schema.Resource {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would split the 2 data sources in different files to follow our convention of one resource or one data source per source file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@nat-henderson
Copy link
Contributor Author

I take back what I said about the website - error repros when I delete both the files I added and use the google.erb from master, so this change probably doesn't cause that issue.

@nat-henderson
Copy link
Contributor Author

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestDataSourceGoogleContainer -timeout 120m
=== RUN   TestDataSourceGoogleContainerRegistryRepository
=== RUN   TestDataSourceGoogleContainerRegistryImage
--- PASS: TestDataSourceGoogleContainerRegistryRepository (0.06s)
--- PASS: TestDataSourceGoogleContainerRegistryImage (0.08s)
PASS
ok      github.com/terraform-providers/terraform-provider-google/google 0.086s
02:18 $ make vet
go vet .
✔

@nat-henderson nat-henderson merged commit d8aea17 into hashicorp:master Jan 18, 2018
@nat-henderson nat-henderson deleted the container-registry branch January 18, 2018 02:47
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
New, very simple data sources for GCR Repo and Image, plus docs.
modular-magician pushed a commit to modular-magician/terraform-provider-google that referenced this pull request Sep 27, 2019
@ghost
Copy link

ghost commented Mar 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GCP container-registry
3 participants