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

Add test and alias for BigQuery Dataset name #1505

Merged
merged 7 commits into from
Mar 13, 2019

Conversation

slevenick
Copy link
Contributor

Add test and alias for BigQuery Dataset name.
The Dataset response resource does not have a field named name, and uses a property on the nested dataset_reference object instead. Add custom method to access this property via name method call.


[all]

[terraform]

[terraform-beta]

[ansible]

[inspec]

@slevenick slevenick force-pushed the inspec-dataset-name-alias branch from 20c3904 to 2e773d9 Compare March 13, 2019 16:44
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 2e773d9.

Pull request statuses

No diff detected in terraform-provider-google-beta.
No diff detected in terraform-google-conversion.
No diff detected in terraform-provider-google.
No diff detected in Ansible.

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: modular-magician/inspec-gcp#125

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, fe6957a.

Pull request statuses

No diff detected in terraform-provider-google-beta.
No diff detected in terraform-google-conversion.
No diff detected in terraform-provider-google.
No diff detected in Ansible.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

lastModifiedTime: !ruby/object:Overrides::Inspec::PropertyOverride
exclude_plural: true
additional_functions: |
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be overkill, but what do you think about either:

  1. Having additional_functions take in a list of Ruby files
  2. Using the compile() function to add a Ruby file.

That way, you can put this Ruby code in a Ruby file that IDEs, Rubocop + the like can test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to using compile

@@ -23,7 +23,8 @@ class PropertyOverride < Overrides::PropertyOverride
def self.attributes
[
:name_from_self_link, # Set to convert self link to name
:exclude_plural
:exclude_plural,
Copy link
Contributor

Choose a reason for hiding this comment

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

Almost certainly overkill, but fyi:

Ansible has two sets of overrides (one for facts modules, one for regular).

In theory, you could do the same with plural/singular resources + just use the regular exclude field.

Again, almost certainly overkill.

@@ -67,7 +67,7 @@ class <%= object.name -%> < GcpResourceBase

def parse
<%
parse_code = object.all_user_properties.map do |prop|
parse_code = object.all_user_properties.reject(&:exclude_reader).map do |prop|
Copy link
Contributor

Choose a reason for hiding this comment

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

Why wouldn't you just exclude the property altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I tried that at one point, but had a bug and forgot to come back to it after I fixed it.

Removed the override and excluded the name field

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 63545ef.

Pull request statuses

No diff detected in terraform-provider-google-beta.
No diff detected in terraform-google-conversion.
No diff detected in terraform-provider-google.
No diff detected in Ansible.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 4f8a6ca.

Pull request statuses

No diff detected in terraform-provider-google-beta.
No diff detected in terraform-google-conversion.
No diff detected in terraform-provider-google.
No diff detected in Ansible.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, ebe90d6.

Pull request statuses

No diff detected in terraform-provider-google-beta.
No diff detected in terraform-google-conversion.
No diff detected in terraform-provider-google.
No diff detected in Ansible.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician modular-magician force-pushed the inspec-dataset-name-alias branch from ebe90d6 to ccf4821 Compare March 13, 2019 19:45
@modular-magician modular-magician merged commit 3569d8d into master Mar 13, 2019
@slevenick slevenick deleted the inspec-dataset-name-alias branch April 15, 2019 19:41
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.

4 participants