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

[bug fix] Fix scope issue #624

Merged
merged 2 commits into from
Nov 13, 2019
Merged

[bug fix] Fix scope issue #624

merged 2 commits into from
Nov 13, 2019

Conversation

dturn
Copy link
Contributor

@dturn dturn commented Nov 13, 2019

What are you trying to accomplish with this PR?
Fix a bug

Traceback (most recent call last):
	10: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/concurrency.rb:13:in `block (2 levels) in split_across_threads'
	 9: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/concurrency.rb:13:in `each'
	 8: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_watcher.rb:56:in `block in sync_resources'
	 7: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/kubernetes_resource/pod.rb:31:in `sync'
	 6: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/kubernetes_resource.rb:173:in `sync'
	 5: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_cache.rb:18:in `get_instance'
	 4: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_cache.rb:46:in `use_or_populate_cache'
	 3: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_cache.rb:46:in `synchronize'
	 2: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_cache.rb:48:in `block in use_or_populate_cache'
	 1: from /usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/resource_cache.rb:54:in `fetch_by_kind'
/usr/lib/ruby-shopify/ruby-shopify-2.6.4/lib/ruby/gems/2.6.0/gems/kubernetes-deploy-0.31.0/lib/krane/task_config.rb:14:in `global_kinds': uninitialized constant Krane::TaskConfig::ClusterResourceDiscovery (NameError)

How is this accomplished?
More strictly scope a class

What could go wrong?
We have test coverage on that line, I don't actually understand the cause, we should figure out how to reproduce it before merging.

Copy link
Contributor

@timothysmith0609 timothysmith0609 left a comment

Choose a reason for hiding this comment

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

:shipit: Assuming you've tested this irl

@dturn dturn marked this pull request as ready for review November 13, 2019 19:03
@dturn
Copy link
Contributor Author

dturn commented Nov 13, 2019

The reason this doesn't impact Krane run is that krane cli pulls in all of krane via https://github.com/Shopify/kubernetes-deploy/blob/master/lib/krane/cli/krane.rb#L3. We (in a subsequent PR) need to change that to be krane/common

@dturn dturn merged commit 9d7bc2b into master Nov 13, 2019
@dturn dturn deleted the fix-class-scope branch November 13, 2019 19:11
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.

None yet

2 participants