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

Remove PAT dependencies #8147

Merged
merged 2 commits into from
Apr 23, 2024
Merged

Remove PAT dependencies #8147

merged 2 commits into from
Apr 23, 2024

Conversation

hallipr
Copy link
Member

@hallipr hallipr commented Apr 23, 2024

@@ -1,57 +0,0 @@
.create-or-alter function with (folder='users/kojamroz', docstring='Build logs for specific ADO job in the specs pipelines builds. See source for details.') SpecsPipelinesBuildsJobLogs(jobName:string, start:datetime, end:datetime, buildId:string="*")
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik Apr 23, 2024

Choose a reason for hiding this comment

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

I would love if we could avoid deleting stuff in my users/kojamroz folder. I added these with the intent of being backup sources. So not sure why they have to be deleted?

The PR says "Remove PAT dependencies" but these sources do not use PATs?

Copy link
Member Author

Choose a reason for hiding this comment

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

The deployment scripts weren't run in previous PRs. They gather all of the kusto files in the infrastructure folder and deploy them as part of the bicep deployment to either test, staging or production. So, committing kusto scripts to the kusto folder should have the intent of putting objects in the production database.

Copy link
Member Author

Choose a reason for hiding this comment

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

For this file, SpecsPipelinesBuildsJobLogs.kql, the cross cluster table access breaks deployment to test, where a fresh cluster is provisioned in Playground without access to the prod cluster.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hallipr how about we move these sources to some location that won't be picked up by any automation? Can you do that in this PR instead of deleting these files?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure they belong in pipeline-witness project if they're not part of the product. Perhaps just add them to the wiki

Copy link
Member Author

Choose a reason for hiding this comment

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

If a process depends on the functions in the prod database, we should fix them so they're deployable to test and staging. Otherwise, we should add them to the Kusto Query Catalog

Copy link
Contributor

@konrad-jamrozik konrad-jamrozik Apr 23, 2024

Choose a reason for hiding this comment

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

@hallipr you mention this is broken due to "cross-cluster". But in fact this function is using only one cluster. I spelled it out to make it explicit. If we would remove the cluster prefix would it start to work? If so, how can I ensure it is uploaded both to PROD and PPE/TEST clusters? Can you point me to relevant pipeline so I can experiment with fixing the query?

In any case, feel free to delete the code from this PR for now. If I manage to fix it I will create another PR.

In the meantime I added them to:
https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/1144/Kusto-by-kojamroz

Copy link
Member Author

Choose a reason for hiding this comment

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

@konrad-jamrozik, the pipeline is here:
https://dev.azure.com/azure-sdk/internal/_build?definitionId=1716&_a=summary

I recently updated it to do full bicep deployment in ci, so we should see deployment breaks as soon as the code hits main. You can also run the pipeline against your branch or PR and only deploy to Staging to test the kql bundle / bicep deployment

Copy link
Member Author

Choose a reason for hiding this comment

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

You can test the deployment by running /tools/pipeline-witness/infrastructure/deploy.ps1 -target test. This will deploy to Playground

@hallipr hallipr force-pushed the users/pahallis/witness-bicep branch from 6b7d35e to 91f667f Compare April 23, 2024 21:39
@hallipr
Copy link
Member Author

hallipr commented Apr 23, 2024

/check-enforcer override

No PR pipelines exist for pipeline-witness

@hallipr hallipr enabled auto-merge (squash) April 23, 2024 21:48
@hallipr hallipr merged commit 8a653ed into main Apr 23, 2024
4 checks passed
@hallipr hallipr deleted the users/pahallis/witness-bicep branch April 23, 2024 21:48
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