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

chore: basic object tracking part 2 #3214

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak commented Nov 21, 2024

follow up to #3205

Changes

  • applied comments from the previous pr (added test in resource and account locator key was changed to struct{}, etc.)
  • applied tracking wrappers to every resource that has implemented context method (in old ones we create context ourselves; I can go and add the context there too), also imports implemented as passthrough were skipped.
  • added missing resource names to resource type
  • added entry to our v1 guidelines to add wrapper around terraform functions
  • modified test client for information schema a bit

Coming up

  • data sources (after pr that adds data source names similar to the resrouces.Resource interface)

Copy link

Integration tests failure for efd7e03299256666a9e107cc91c8da75466d039e

Copy link
Collaborator

@sfc-gh-jmichalak sfc-gh-jmichalak left a comment

Choose a reason for hiding this comment

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

Please also check other resources if they have import tracking set up.

}

func (c *InformationSchemaClient) GetQueryTagByQueryId(t *testing.T, queryId string) string {
func (c *InformationSchemaClient) GetQueryHistoryByQueryId(t *testing.T, limit int, queryId string) QueryHistory {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can agree on a default limit here and avoid specifying it in each call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would leave it as a value to specify as I changed this a lot during testing and we should aim for as little number as possible

Copy link

Integration tests failure for 5bc77fce87f42a4b98cb46142574b74ebf43c313

Comment on lines +43 to +48
if metadata, err := tracking.ParseMetadata(history.QueryText); err == nil {
if expectedMetadata == metadata && strings.Contains(history.QueryText, query) {
return true
}
}
return false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
if metadata, err := tracking.ParseMetadata(history.QueryText); err == nil {
if expectedMetadata == metadata && strings.Contains(history.QueryText, query) {
return true
}
}
return false
metadata, err := tracking.ParseMetadata(history.QueryText)
return err == nil &&
expectedMetadata == metadata &&
strings.Contains(history.QueryText, query)

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review November 22, 2024 15:22
Copy link

Integration tests cancelled for ab0095a4a28b16ae49560ef7c86865a3c0d7e1ff

@sfc-gh-jcieslak sfc-gh-jcieslak merged commit e44f2e1 into main Nov 25, 2024
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the basic-object-tracking-part2 branch November 25, 2024 09:07
sfc-gh-jmichalak pushed a commit that referenced this pull request Nov 26, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.99.0](v0.98.0...v0.99.0)
(2024-11-26)


### 🎉 **What's new:**

* Add tags data source
([#3211](#3211))
([8907d9d](8907d9d))
* Tag resource v1
([#3197](#3197))
([77b3bf0](77b3bf0))
* Tasks v1 readiness
([#3222](#3222))
([e2284d9](e2284d9))


### 🔧 **Misc**

* Add support for usage tracking to data sources
([#3224](#3224))
([8210bb8](8210bb8))
* Add usage tracking for the rest of the resources and fix views
([#3223](#3223))
([231f653](231f653))
* Basic object tracking
([#3205](#3205))
([1f0dc94](1f0dc94))
* basic object tracking part 2
([#3214](#3214))
([e44f2e1](e44f2e1))
* Improve tags integration tests
([#3193](#3193))
([7736e0a](7736e0a))
* parser and secret tests
([#3192](#3192))
([5ec9c86](5ec9c86))
* Storage integration with custom protocol
([#3213](#3213))
([a3a44ae](a3a44ae))
* Unskip auth config tests
([#3180](#3180))
([46ab142](46ab142))


### 🐛 **Bug fixes:**

* Small fixes and adjustments
([#3226](#3226))
([9f67457](9f67457))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
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.

3 participants