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

Specify the orverride import order #559

Merged
merged 5 commits into from
Oct 15, 2018

Conversation

chrisst
Copy link
Contributor

@chrisst chrisst commented Oct 13, 2018

There are some situations where the order that resources are overridden is
critical. We have gotten lucky up until now that a strategy of "deepest first"
was good enough. This will let us force the order when two or more resources
depend on a specific ordering.


[all]

[terraform]

[puppet]

[puppet-bigquery]

[puppet-compute]

[puppet-container]

[puppet-dns]

[puppet-logging]

[puppet-pubsub]

[puppet-resourcemanager]

[puppet-sql]

[puppet-storage]

[chef]

[chef-compute]

[chef-container]

[chef-dns]

[chef-logging]

[chef-spanner]

[chef-sql]

[chef-storage]

[ansible]

There are some situations where the order that resources are overridden is
critical. We have gotten lucky up until now that a strategy of "deepest first"
was good enough. This will let us force the order when two or more resources
depend on a specific ordering.
@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!
I checked the downstream repositories (see README.md for which ones I can write to), and none of them seem to have any changes.

Once this PR is approved, you can feel free to merge it without taking any further steps.

@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!
I checked the downstream repositories (see README.md for which ones I can write to), and none of them seem to have any changes.

Once this PR is approved, you can feel free to merge it without taking any further steps.

@@ -20,6 +20,16 @@ module Provider
class PropertyOverride < Api::Object
attr_reader :new_type

# Ordering of imports can be important so this property can make the
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite know what you mean by "ordering of imports".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I meant to say "ordering of overrides". Such that if 1 override is applied before a different one (in this case if Image is applied before SourceImage) then MM will break. This is because Source image is adding properties that the Image override depends on.

@@ -20,6 +20,16 @@ module Provider
class PropertyOverride < Api::Object
attr_reader :new_type

# Ordering of imports can be important so this property can make the
Copy link
Contributor

Choose a reason for hiding this comment

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

s/can make/makes/

# with the lowest number getting imported first.
# The default is equal to the negative depth of the metric. This makes
# sure that more deeply nested fields happen first.
# eg:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe "eg, for the compute.disk resource:"

@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!
I checked the downstream repositories (see README.md for which ones I can write to), and none of them seem to have any changes.

Once this PR is approved, you can feel free to merge it without taking any further steps.

@modular-magician modular-magician merged commit ec787c0 into GoogleCloudPlatform:master Oct 15, 2018
@chrisst chrisst deleted the override-order branch October 15, 2018 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants