Skip to content

fix(azure): Use lowercase namespaces#5789

Merged
yevgenypats merged 3 commits intomainfrom
fix/azure_namespace
Dec 19, 2022
Merged

fix(azure): Use lowercase namespaces#5789
yevgenypats merged 3 commits intomainfrom
fix/azure_namespace

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

This should fix some of the missing tables we were seeing in azure. apparently namespaces are not consistent and can be lowered case, camel cased, something else... So we normalize them all to lower-case (given they are case insensitive) on the client side.

@yevgenypats
Copy link
Copy Markdown
Contributor Author

Closes:
#5761
#5683

@vercel
Copy link
Copy Markdown

vercel bot commented Dec 19, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cloudquery-web 🔄 Building (Inspect) Dec 19, 2022 at 8:39AM (UTC)

Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Looks good but I think this fixes #5760 and #5761, but not #5683 as for #5683 we still need #5758.

Also could we drop the namespace multiplexer altogether? We don't use the namespace in the List API calls. I tried it for a few API calls in #5758 and it doesn't generate a lot of errors (only one errored out with a "not registered service").

The last comment is more a product decision, dropping the namespace multiplexer will make the Azure plugin consistent with GCP's default behavior

@yevgenypats
Copy link
Copy Markdown
Contributor Author

yevgenypats commented Dec 19, 2022

I don't think we need to drop the multiplexer because otherwise azure will hit disabled APIs and this will be (really) slow. we were actually trying to implement this in GCP but with no success so far due to some issues with their list disabled APIs.

Regarding the issue - Im not sure what the customer is reporting about - is they are not seeing anything at all? specific table. I think the issue needs more elaboration.

@cq-bot cq-bot added the website label Dec 19, 2022
@github-actions
Copy link
Copy Markdown

This PR has the following changes to source plugin(s) tables:

  • ⚠️ BREAKING CHANGE: Table azure_sql_deleted_servers was removed

@yevgenypats
Copy link
Copy Markdown
Contributor Author

This PR has the following changes to source plugin(s) tables:

  • ⚠️ BREAKING CHANGE: Table azure_sql_deleted_servers was removed

@erezrokah Can we not mark it as breaking. This table was never working.

@yevgenypats
Copy link
Copy Markdown
Contributor Author

This PR has the following changes to source plugin(s) tables:

  • ⚠️ BREAKING CHANGE: Table azure_sql_deleted_servers was removed

@erezrokah Can we not mark it as breaking? This table was never working.

@yevgenypats yevgenypats merged commit b43e1bd into main Dec 19, 2022
@yevgenypats yevgenypats deleted the fix/azure_namespace branch December 19, 2022 08:50
@erezrokah
Copy link
Copy Markdown
Member

@yevgenypats This comment is just FYI, as long as the PR title/body doesn't say it's breaking it won't bump the major version

@erezrokah
Copy link
Copy Markdown
Member

I'm not sure what the customer is reporting about

There are 2 separate issues:

  1. We don't provide any information when we filter the tables based on the provider being registered, so users can't know why the tables are not synced. This is fixed in feat(azure): Add warning message when a namespace is not registered #5758
  2. We were using the wrong namespaces (Bug(azure): Mismatch between auto generated namespaces and actual namespaces #5760) which this PR closes

kodiakhq bot pushed a commit that referenced this pull request Dec 20, 2022
🤖 I have created a release *beep* *boop*
---


## [2.2.0](plugins-source-azure-v2.1.1...plugins-source-azure-v2.2.0) (2022-12-20)


### Features

* **azure:** Add storage_containers ([#5759](#5759)) ([18003e9](18003e9))


### Bug Fixes

* **azure:** Remove extra `fmt.Println` ([#5756](#5756)) ([4c588b1](4c588b1))
* **azure:** Use lowercase namespaces ([#5789](#5789)) ([b43e1bd](b43e1bd))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v1.12.5 ([#5661](#5661)) ([b354b8a](b354b8a))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v1.12.6 ([#5790](#5790)) ([8e2663c](8e2663c))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v1.12.7 ([#5797](#5797)) ([15da529](15da529))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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