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

fix: workspace config connection as map instead of slice #12

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

achettyiitr
Copy link
Member

@achettyiitr achettyiitr commented Jan 23, 2024

Description

  • Connections are coming as maps instead of slices.

Linear Ticket

  • Resolves PIPE-714

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@achettyiitr achettyiitr requested a review from fxenik January 23, 2024 12:02
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c84890a) 97.74% compared to head (550173e) 97.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #12   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files           7        7           
  Lines         177      177           
=======================================
  Hits          173      173           
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -16,7 +16,7 @@ type WorkspaceConfig struct {
Settings *Settings `json:"settings"`
Sources map[string]*Source `json:"sources"`
Destinations map[string]*Destination `json:"destinations"`
Connections []*Connection `json:"connections"`
Connections map[string]*Connection `json:"connections"`
Copy link
Contributor

Choose a reason for hiding this comment

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

@fxenik do you think we can come up with a way to test these? an integration test perhaps?

Copy link
Member

Choose a reason for hiding this comment

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

If we are spending any effort on testing these automatically, I think we should do it in a way that makes sense for the whole org. We should resume the work for provisioning a disposable control plane and then write integration tests for the SDK to ensure it is retrieving/parsing data correctly. I'll have a chat with Sumanth today to see how we can find bandwidth for that.

Copy link
Contributor

@fracasula fracasula left a comment

Choose a reason for hiding this comment

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

Approved with the plan to revisit with kind once we got the helm charts working.

@achettyiitr can you create a task for the follow up please?

@achettyiitr
Copy link
Member Author

achettyiitr commented Jan 30, 2024

Approved with the plan to revisit with kind once we got the helm charts working.

@achettyiitr can you create a task for the follow up please?

https://linear.app/rudderstack/issue/PIPE-734/integration-tests-using-kind-for-cp-sdk

@achettyiitr achettyiitr merged commit 18b146c into main Jan 30, 2024
12 checks passed
@achettyiitr achettyiitr deleted the fix.workspace-config-connection-map branch January 30, 2024 13:28
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