Skip to content

[rdbms-connect] Added file execution command#3257

Merged
Juliehzl merged 12 commits intoAzure:masterfrom
DaeunYim:daeunyim/file-execution
May 25, 2021
Merged

[rdbms-connect] Added file execution command#3257
Juliehzl merged 12 commits intoAzure:masterfrom
DaeunYim:daeunyim/file-execution

Conversation

@DaeunYim
Copy link
Copy Markdown
Contributor

@DaeunYim DaeunYim commented Apr 13, 2021 •
edited
Loading

  • Made new command group 'execute'
  • Added sql file execution command

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

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 PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@DaeunYim DaeunYim requested a review from Juliehzl as a code owner April 13, 2021 05:40
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Apr 13, 2021

rdbms-connect

@yonzhan yonzhan requested a review from evelyn-ys April 13, 2021 07:43
@yonzhan yonzhan added this to the S186 milestone Apr 13, 2021
@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented Apr 21, 2021

Can I get any reviews for this PR?

Juliehzl
Juliehzl reviewed Apr 21, 2021
View reviewed changes
Juliehzl
Juliehzl reviewed Apr 21, 2021
View reviewed changes
Juliehzl
Juliehzl reviewed Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

Could you add tests for new commands?

@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented Apr 23, 2021

Could you add tests for new commands?

Added the tests!

Juliehzl
Juliehzl reviewed Apr 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

Do you forget to upload recording file for your test? With live test change, you recording file also need to re-record and please also remember to upload yaml file. Otherwise, CI will fail.

@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented Apr 27, 2021

Can I get your input on CI failure? Looks like other extensions tests are failing..
e.g., storage-preview

@Juliehzl
Copy link
Copy Markdown
Contributor

Juliehzl commented Apr 28, 2021

Can I get your input on CI failure? Looks like other extensions tests are failing..
e.g., storage-preview

Hi @DaeunYim, could you rebase latest master branch and CI will bypass the check for other extensions?

@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented Apr 29, 2021

@Juliehzl Thanks a lot! It worked and CI passed :)

@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented May 5, 2021

Can we resume releasing this extension again?

@Juliehzl
Copy link
Copy Markdown
Contributor

Juliehzl commented May 12, 2021

Can we resume releasing this extension again?

Do you want to release a new version for your extension? If yes, please upgrade version in setup.py and add release note in history.rst.

@yonzhan yonzhan modified the milestones: S186, S187 May 12, 2021
@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented May 17, 2021

Can we resume releasing this extension again?

Do you want to release a new version for your extension? If yes, please upgrade version in setup.py and add release note in history.rst.

Done!

@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented May 18, 2021

@Juliehzl
Could you please help me with this..?
I am having verify extension index error. It says

ERROR: test_metadata (main.TestIndex)

Traceback (most recent call last):
File "./scripts/ci/test_index.py", line 142, in test_metadata
self.whl_cache_dir, self.whl_cache)
File "/home/vsts/work/1/s/scripts/ci/util.py", line 94, in get_whl_from_url
for chunk in r.iter_content(chunk_size=1024):
UnboundLocalError: local variable 'r' referenced before assignment

Looks like whl file is not found. Do you have any recommendation I can do to fix this..?

Juliehzl
Juliehzl reviewed May 21, 2021
View reviewed changes
Juliehzl
Juliehzl reviewed May 21, 2021
View reviewed changes
Juliehzl
Juliehzl reviewed May 21, 2021
View reviewed changes
@DaeunYim
Copy link
Copy Markdown
Contributor Author

DaeunYim commented May 24, 2021

Is there any other issues left for this one??

@Juliehzl Juliehzl merged commit 462e237 into Azure:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Juliehzl Juliehzl Juliehzl approved these changes

@evelyn-ys evelyn-ys Awaiting requested review from evelyn-ys

Assignees

@Juliehzl Juliehzl

Labels

None yet

Projects

None yet

Milestone

S187

Development

Successfully merging this pull request may close these issues.

3 participants

@DaeunYim @yonzhan @Juliehzl