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

Added Telemetry and fixed Template Upload error #25991

Merged
merged 13 commits into from
Oct 28, 2024

Conversation

sahuroshan
Copy link
Contributor

This PR fixes 3 things.

  1. Added Telemetry for Target provisioning.

  2. Instead of pasing skurecommendation file path , now we are passing just target type to the getarmtemplate API in toolsservice.

  3. There was an intermitent issue while uploading ARM templates , the upload was failing sometimes. To fix this we are now generating Account SAS , earlier we were generating container SAS. This was problem with azure/storage-blob node module.
    Found similar isue here - https://learn.microsoft.com/en-us/answers/questions/334786/azure-blob-storage-fails-to-authenticate-make-sure
    image

@coveralls
Copy link

coveralls commented Oct 22, 2024

Pull Request Test Coverage Report for Build 11504358086

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.001%) to 41.711%

Files with Coverage Reduction New Missed Lines %
src/sql/workbench/services/connection/browser/connectionDialogWidget.ts 1 73.44%
Totals Coverage Status
Change from base Build 11467144417: -0.001%
Covered Lines: 30719
Relevant Lines: 68916

💛 - Coveralls

@@ -263,6 +263,7 @@ export const IMPORT_PERFORMANCE_DATA_DIALOG_OPEN_FOLDER = localize('sql.migratio
export const UPLOAD_TEMPLATE_TO_AZURE = localize('sql.migration.target.provisioning.upload.to.azure', "Deploy to Azure");
export const SAVE_TO_DEVICE = localize('sql.migration.target.provisioning.generate.template', "Save to device");
export const COPY_TO_CLIPBOARD = localize('sql.migration.target.provisioning.copy.to.clipboard', "Copy to clipboard");
export const ARM_TEMPLATE_GENERATE_FAILED = localize('sql.migration.target.provisioning.copy.to.clipboard', "Failed to generate ARM template");
Copy link
Contributor

@Bhavikshah123 Bhavikshah123 Oct 23, 2024

Choose a reason for hiding this comment

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

Change it to appropriate helper string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@Bhavikshah123 Bhavikshah123 left a comment

Choose a reason for hiding this comment

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

HAs this been tested in all platforms?

@sahuroshan
Copy link
Contributor Author

HAs this been tested in all platforms?

yes working on mac,windows,linux.

@sahuroshan sahuroshan merged commit 660f87d into main Oct 28, 2024
11 of 12 checks passed
@sahuroshan sahuroshan deleted the roshansahu/telemetryAndUploadTemplateFix branch October 28, 2024 02:26
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