Skip to content

Add external language support#82

Merged
JonathanZhu11 merged 29 commits into
masterfrom
JonathanZhu11/extensions
Oct 27, 2020
Merged

Add external language support#82
JonathanZhu11 merged 29 commits into
masterfrom
JonathanZhu11/extensions

Conversation

@JonathanZhu11

@JonathanZhu11 JonathanZhu11 commented Sep 24, 2020

Copy link
Copy Markdown
Contributor

With SQL Server 2019 CU3+ there are "language extensions" which are custom runtimes of R or Python that the user can create. This PR makes sqlmlutils work with sp_execute_external_script calls to R/Python that are called something else because they are an external language.

We need to disable a few tests because they are too slow to run in the CI. I will need to rework them to be faster in a later PR.

https://github.com/microsoft/sql-server-language-extensions

There are also some minor changes for addressing packaging bugs: we have a max wheel version (which still has pep425tags) and we use sysconfig for get_platform.

@Aniruddh25

Aniruddh25 commented Oct 7, 2020

Copy link
Copy Markdown

suggestion, if not already done, please create a task to reenable the disabled tests #Closed

@Aniruddh25

Aniruddh25 commented Oct 7, 2020

Copy link
Copy Markdown

I notice the sqlmlutils package version on Python is different than in R. Is that intentional ? #Closed

Comment thread Python/sqlmlutils/sqlpythonexecutor.py Outdated
Comment thread Python/sqlmlutils/packagemanagement/sqlpackagemanager.py Outdated
Comment thread Python/sqlmlutils/packagemanagement/servermethods.py
Comment thread Python/tests/package_management_pypi_test.py Outdated
Comment thread Python/tests/package_management_pypi_test.py Outdated
Comment thread Python/tests/package_management_pypi_test.py
@Aniruddh25

Aniruddh25 commented Oct 7, 2020

Copy link
Copy Markdown
    assert "exists on server. Set upgrade to True" in output.getvalue()

make sure to check version even in this new test and verify we get old_version ?
basically, the following check_version() function from the test below can be made a common function with packagename as parameter and both tests can call this.

    def check_version(packagename):
        import packagename as cp
        return cp.__version__

    oldversion = pyexecutor.execute_function_in_sql(check_version) #Closed

Refers to: Python/tests/package_management_pypi_test.py:104 in 428704f. [](commit_id = 428704f, deletion_comment = False)

Comment thread Python/sqlmlutils/sqlpythonexecutor.py
@Aniruddh25

Aniruddh25 commented Oct 8, 2020

Copy link
Copy Markdown

pyexecutor = SQLPythonExecutor(connection)

Looks to me these tests haven't been updated to use myPy yet, correct ?

You can add another task to add new tests using myPy/myR itself #Closed


Refers to: Python/tests/package_management_pypi_test.py:15 in 428704f. [](commit_id = 428704f, deletion_comment = False)

@Aniruddh25 Aniruddh25 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🕐

@JonathanZhu11

Copy link
Copy Markdown
Contributor Author

It's... sort of intentional. I was planning to update R to version 1.0 when we got it into CRAN but CRAN is much harder to get into than PyPI (they have a lot more requirements). Also, matching the two versions doesn't necessarily make sense - if I make a bug fix in one language that shouldn't update the version in the other.

I should probably make them the same major version though.

Also, this change might be a major version change, so I may update both of them to v2.0.


In reply to: 705138493 [](ancestors = 705138493)

@JonathanZhu11

Copy link
Copy Markdown
Contributor Author

pyexecutor = SQLPythonExecutor(connection)

i have made a task


In reply to: 705259892 [](ancestors = 705259892)


Refers to: Python/tests/package_management_pypi_test.py:15 in 428704f. [](commit_id = 428704f, deletion_comment = False)

Comment thread Python/tests/package_management_pypi_test.py
Comment thread Python/sqlmlutils/packagemanagement/pipdownloader.py
Comment thread Python/tests/conftest.py Outdated
Comment thread R/tests/testthat/helper-sqlPackage.R

@karthikvenkataramaiah karthikvenkataramaiah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:shipit:

@Aniruddh25 Aniruddh25 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:shipit:

@JonathanZhu11 JonathanZhu11 merged commit c6ff084 into master Oct 27, 2020
@JonathanZhu11 JonathanZhu11 deleted the JonathanZhu11/extensions branch March 15, 2021 22: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