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.

Sign up for GitHub

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

Jump to bottom

Add support for managed MySQL Flexible server #6411

Merged
merged 9 commits into from
Aug 28, 2023
Merged

Add support for managed MySQL Flexible server #6411

merged 9 commits into from
Aug 28, 2023

Conversation

yash-nisar
Copy link
Contributor

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@azure-client-tools-bot-prd
Copy link

Hi @yash-nisar,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Jun 15, 2023

Thank you for your contribution! We will review the pull request and get back to you soon.

@yash-nisar
Copy link
Contributor Author

It takes ~6mins to provision a flexible mysql server from the command line so I don't think it's feasible to add ITs:
image

@yash-nisar
Copy link
Contributor Author

Verified the binding works:
image

@yash-nisar
Copy link
Contributor Author

Also could see env vars on curling the app:

AZURE_MYSQL_DATABASE=XX
AZURE_MYSQL_HOST=XX
AZURE_MYSQL_PASSWORD=XX
AZURE_MYSQL_PORT=XX
AZURE_MYSQL_SSL=XX
AZURE_MYSQL_USERNAME=XX

Command to bind:

az containerapp update -n appName -g rgName --bind mysqlName:mysqlconnName,username=XX,password=XX,database=XX

Greedygre
Greedygre reviewed Jun 16, 2023
View reviewed changes
src/containerapp/azext_containerapp/_managed_service_utils.py
return {"linker_name": binding_name, "parameters": parameters, "resource_id": resource_id}


class ManagedMySQLFlexibleUtils:
Copy link
Contributor

@Greedygre Greedygre Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Please add tests for command
az containerapp create/update --bind to test the MySQL service binding.
Otherwise LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

test function added, thank you.

@zhoxing-ms
Copy link
Contributor

Could you please add some test case for this PR change?

@yash-nisar
Copy link
Contributor Author

yash-nisar commented Aug 21, 2023 •
edited
Loading

@bgashirabake is working on the tests and will continue this PR.

Note: Since it takes ~6 mins to provision a flexible mysql server, the runtime of the test is expected to increase.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Aug 22, 2023 •
edited
Loading

️✔️Azure CLI Extensions Breaking Change Test
️✔️Non Breaking Changes

@bgashirabake
Copy link
Contributor

Could you please add some test case for this PR change?

done. could you please take a look when you get the chance. thank you!!

@Greedygre
Copy link
Contributor

Hi @zhoxing-ms @yanzhudd
Could you help to review and merge this PR? Thanks.

@zhoxing-ms zhoxing-ms merged commit 94b2e14 into Azure:main Aug 28, 2023
14 checks passed
bgashirabake added a commit to bgashirabake/azure-cli-extensions that referenced this pull request Aug 30, 2023
@yash-nisar @bgashirabake @zhoxing-ms
Add support for managed MySQL Flexible server (Azure#6411)
960456f 
* Add support for managed MySQL Flexible server

* fix

* add change to history.rst

* test add

* fix duplicate env create cmd

* test rerun

* test update

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py

Co-authored-by: Xing Zhou <[email protected]>

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: bgashirabake <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
bgashirabake added a commit to bgashirabake/azure-cli-extensions that referenced this pull request Aug 30, 2023
@yash-nisar @bgashirabake @zhoxing-ms
Add support for managed MySQL Flexible server (Azure#6411)
6a509e7 
* Add support for managed MySQL Flexible server

* fix

* add change to history.rst

* test add

* fix duplicate env create cmd

* test rerun

* test update

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py

Co-authored-by: Xing Zhou <[email protected]>

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: bgashirabake <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
bgashirabake added a commit to bgashirabake/azure-cli-extensions that referenced this pull request Aug 30, 2023
@yash-nisar @bgashirabake @zhoxing-ms
Add support for managed MySQL Flexible server (Azure#6411)
118c7a3 
* Add support for managed MySQL Flexible server

* fix

* add change to history.rst

* test add

* fix duplicate env create cmd

* test rerun

* test update

* Update src/containerapp/azext_containerapp/tests/latest/test_containerapp_commands.py

Co-authored-by: Xing Zhou <[email protected]>

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: bgashirabake <[email protected]>
Co-authored-by: Xing Zhou <[email protected]>
@yash-nisar yash-nisar deleted the mysql-flexible branch September 6, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgashirabake bgashirabake bgashirabake left review comments

@zhoxing-ms zhoxing-ms zhoxing-ms approved these changes

@Greedygre Greedygre Greedygre approved these changes

@yanzhudd yanzhudd Awaiting requested review from yanzhudd

Assignees

@zhoxing-ms zhoxing-ms

Labels
None yet
Projects
None yet
Milestone
August 2023 (2023-09-05)
Development

Successfully merging this pull request may close these issues.
5 participants
@yash-nisar @yonzhan @zhoxing-ms @bgashirabake @Greedygre