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

AzureMySqlDeployment task update broke Windows agent deployments #14352

Closed
mattboothman opened this issue Feb 7, 2021 · 15 comments
Closed

AzureMySqlDeployment task update broke Windows agent deployments #14352

mattboothman opened this issue Feb 7, 2021 · 15 comments

Comments

@mattboothman
Copy link

Required Information

Entering this information will route you directly to the right team and expedite traction.

Question, Bug, or Feature?
Type: Bug

Enter Task Name: AzureMysqlDeployment

list here (V# not needed):
https://github.com/Microsoft/azure-pipelines-tasks/tree/master/Tasks

Environment

Azure Pipelines

Hosted, WIndows Server 2019, agent version 2.181.1

MySQL Client v8

Issue Description

A recent update to the AzureMysqlDeployment task was recently deployed to UK South. Our MySQL deployments broke emitting error ERROR at line 1: Unknown command '\a'.

Here's the PR that broke the task: #14183

There's a link to a thorough description of the problem: #14183 (comment)

Comment if you would like me to copy/paste the description from the PR to this issue.

@mattboothman
Copy link
Author

I'm not sure why Task: SqlDacpacDeployment is tagged. I've not looked at it, not reporting an issue with that task.

@20shivangi
Copy link
Contributor

20shivangi commented Feb 8, 2021

@mattboothman You are confirmed that this PR broke your task?
cc: @AmrutaKawade

@20shivangi 20shivangi added environment:bug and removed Task: SqlDacpacDeployment environment:need-to-triage Issues need to be triage by environment-deployment team bug labels Feb 8, 2021
@mattboothman
Copy link
Author

@20shivangi yes

@20shivangi
Copy link
Contributor

Will check with Amruta and update the same. Thanks @mattboothman for confirming

@mattboothman
Copy link
Author

Thanks @20shivangi. Please don't just roll back, it'll break the other guys pipeline.

@mattboothman
Copy link
Author

mattboothman commented Feb 8, 2021

Can it be sorted so we can work around the issue, too. The task doesn't really need to check if the file exists, mysql can do and emit its own error:

##[error]Error: No package found with specified pattern C:\agent\_work\12\s\DatabaseScripts\ApplicationDbContext.sql

Similarly, when I do find the right relative path:

-e "source C:\agent\_work\12\DatabaseScripts\ApplicationDbContext.sql;"

Back to square 1!

Can the task please not check the file exists, let MySQL figure that out, and also not mess with the parameters passed into the task?

Here's what I'm passing into it now:

SqlFile: '../DatabaseScripts/ApplicationDbContext.sql'

The task then mangles the path to what it thinks the path should be.

@mattboothman
Copy link
Author

Even using SqlInline, the task interferes with:

sqlInline: 'source ../DatabaseScripts/ApplicationDbContext.sql;'

It mangles to:

-e "source C:\agent\_work\12\s;"

@20shivangi
Copy link
Contributor

We will check on this for sure and get back to you.
Also, if you have any code changes in mind, you can surely raise PR against master and we will be happy to review that one :)

@mattboothman
Copy link
Author

mattboothman commented Feb 8, 2021

@20shivangi thank you.

Unfortunately, i've no idea what I'm doing with TS. If I went poking around, I'm sure I'd break it even more than it is now.

Forgive me for being frank, guys, but this task is a bit pickled. I think what is needed is a V2, get someone who understands MySQL client, and Typescript to do it from scratch. Don't mutate parameters being send in, don't do any checking to see if files exist, just let mysql emit its own errors if we get stuff wrong.

@mattboothman
Copy link
Author

mattboothman commented Feb 8, 2021

Ok, I have my workround working. There are more issues to look at with this task and its documentation:

The workaround is this:

taskNameSelector: 'InlineSqlTask'
sqlInline: 'source ../DatabaseScripts/ApplicationDbContext.sql;'

The workaround involves not following the documentation for taskNameSelector:

image

...by passing what it says is the option for inline scripts for the taskNameSelector (inlineSqlTask) parameter because the ts does a case-sensitive comparison:

sql/MysqlClient.ts:        if( this._azureMysqlTaskParameter.getTaskNameSelector() === 'InlineSqlTask' ) {

The documentation for the taskNameSelector further down the page is also no help:

image

But essentially the workaround succeeds in adding the correct, unmodified -e parameter value (albeit a strangely formatted one):

"-esource ../DatabaseScripts/ApplicationDbContext.sql;"

MySQL client successfully finds the script and executes it.

@20shivangi
Copy link
Contributor

@mattboothman Is the workaround working for you ? If that is the case, can we close the issue ? :D

@mattboothman
Copy link
Author

mattboothman commented Feb 9, 2021

@20shivangi I suppose that depends. Are you guys, and Microsoft, happy with this task being broken and the documentation being wrong/incomplete?

I'm happy to will submit a PR to fix the documentation if you could let me know where/how to do that. Found where /how.

Otherwise, IMO, this ought to be fixed. It's ruined for Windows agents at the moment. Although we can happily work around the break, all that'll happen is someone else'll raise another issue.

@mattboothman
Copy link
Author

@20shivangi I'm not sure if this comment on the docs PR helps to clarify the problem: MicrosoftDocs/azure-devops-docs#10160 (comment)

@20shivangi
Copy link
Contributor

20shivangi commented Feb 10, 2021

@N-Usha Can you have a look at the documentation PR : https://github.com/MicrosoftDocs/azure-devops-docs/pull/10160/files
The bug has been corrected with PR

@AmrutaKawade AmrutaKawade self-assigned this Feb 10, 2021
@20shivangi
Copy link
Contributor

Closing the issue since PR for bug fix and for documentation have been merged. Deployment usually takes 3-4 weeks for rollout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants