#58 Made the pytest-slc plugin using the pyexasol_connection #59

ahsimb · 2024-09-20T13:07:12Z

closes #58

tkilias · 2024-09-22T21:26:10Z

pytest-slc/exasol/pytest_slc/__init__.py

-                                             bucketfs_path=bucketfs_path,
-                                             language_alias=slc_builder.language_alias)
-        deployer.run(container_file=export_slc, alter_system=True, allow_override=True)
+    def func(language_alias: str) -> None:


I guess, you changed it to that, such that we can isolate the aliases per test, if it necessary. I could assume, in case of the deployer tests and the extension tests that could be necessary. Or, did you think of a different reason.

I guess the motivation is described in the related ticket #58:

If an extension test uses both pyexasol_connection fixture from the pytest-extension and upload_slc fixture from the pytest-slc then the container may not get activated for the pyexasol_connection session if the pyexasol_connection fixture is created first. To make sure this doesn't happen the pytest-slc should depend on the pytest-extension and use its pyexasol_connection fixture instead of creating its own connection.

But I have some more questions:

Could we make the function func use the language_alias that is already known to the slc_builder?

Why does the fixture return a function instead of just uploading the SLC?

My question was related to the function and not the Pyexasol connection

We used to take the language_alias from the LanguageContainerBuilder. As we found, the language_alias is not actually required to build a language container; it is only for testing it, which we don't do. So it was removed from the LanguageContainerBuilder. But we do need it for the container deployer. Hence is the upload_slc function. We can provide the language_alias there.

But good point , maybe we need a fixture that returns a function that activates the SLC for a connection

We used to take the language_alias from the Builder ... language_alias is not actually required to build a language container ... But for the deployer. ...

Ah - very good, thank you!

Decision:

Rename fixture upload_slc to deploy_slc

Add fixture deployed_slc which uses a dummy fixture for alias which needs be overwritten by the extension

the function returned by deploy_slc fixture uses a state variable stored in deploy_slc fixture to upload the slc only once and only activate it in subsequent calls

pytest-slc/doc/changes/changes_0.3.0.md

ckunki · 2024-09-23T06:16:12Z

pytest-slc/exasol/pytest_slc/__init__.py

-                                             bucketfs_path=bucketfs_path,
-                                             language_alias=slc_builder.language_alias)
-        deployer.run(container_file=export_slc, alter_system=True, allow_override=True)
+    def func(language_alias: str) -> None:


I guess the motivation is described in the related ticket #58:

If an extension test uses both pyexasol_connection fixture from the pytest-extension and upload_slc fixture from the pytest-slc then the container may not get activated for the pyexasol_connection session if the pyexasol_connection fixture is created first. To make sure this doesn't happen the pytest-slc should depend on the pytest-extension and use its pyexasol_connection fixture instead of creating its own connection.

ckunki · 2024-09-23T06:21:34Z

pytest-slc/test/integration/pytest_slc_test.py

@@ -24,7 +24,7 @@ def assert_udf_running(conn: pyexasol.ExaConnection):
    with temp_schema(conn) as schema:
        udf_name = 'TEST_UDF'
        udf_create_sql = (
-            f'CREATE OR REPLACE {{LANGUAGE_ALIAS}} SCALAR SCRIPT "{{schema}}"."{{udf_name}}"() '
+            f'CREATE OR REPLACE {LANGUAGE_ALIAS} SCALAR SCRIPT "{{schema}}"."{{udf_name}}"() '


I am not 100% sure, but shouldn't all parameters be enclosed only in single braces to allow f-string substitution?

Suggested change

f'CREATE OR REPLACE {LANGUAGE_ALIAS} SCALAR SCRIPT "{{schema}}"."{{udf_name}}"() '

f'CREATE OR REPLACE {LANGUAGE_ALIAS} SCALAR SCRIPT "{schema}"."{udf_name}"() '

This is a piece of code, which itself uses f-strings. This is the case for the schema and udf_name. In this code, the LANGUAGE_ALIAS is a constant which we insert externally, also using an f-string. Basically, there is an external and internal f-string.

pytest-slc/test/integration/pytest_slc_test.py

ckunki · 2024-09-23T06:32:28Z

pytest-slc/exasol/pytest_slc/__init__.py

-                                             bucketfs_path=bucketfs_path,
-                                             language_alias=slc_builder.language_alias)
-        deployer.run(container_file=export_slc, alter_system=True, allow_override=True)
+    def func(language_alias: str) -> None:


But I have some more questions:

Could we make the function func use the language_alias that is already known to the slc_builder?

Why does the fixture return a function instead of just uploading the SLC?

Co-authored-by: Christoph Kuhnke <[email protected]>

ckunki · 2024-09-23T13:51:39Z

pytest-slc/README.md

+Here, we override the default `language_alias` fixture providing a "meaningful" name. The language container will be
+activated with this language alias. Note, that by default the test will run twice - once for each backend.


Suggested change

Here, we override the default `language_alias` fixture providing a "meaningful" name. The language container will be

activated with this language alias. Note, that by default the test will run twice - once for each backend.

The example test case overrides two fixtures that are used by fixture `deployed_slc` from this plugin:

* Fixture `language_alias` to provide a meaningful name.

* and fixture `slc_builder` to define the SLC to be used in the example test case.

The language container will be activated with value returned by fixture `language_alias`.

Note, that by default the test will run twice - once for each backend.

#58 Made the plugin using the pyexasol_connection

d72ddc2

ahsimb added the bug Unwanted / harmful behavior label Sep 20, 2024

ahsimb requested a review from ckunki September 20, 2024 13:07

ahsimb self-assigned this Sep 20, 2024

#58 Made a change in the upload_slc fixture

a68a312

tkilias reviewed Sep 22, 2024

View reviewed changes

ckunki requested changes Sep 23, 2024

View reviewed changes

ckunki previously approved these changes Sep 23, 2024

View reviewed changes

ahsimb dismissed ckunki’s stale review via 4854404 September 23, 2024 07:36

Apply suggestions from code review

4854404

Co-authored-by: Christoph Kuhnke <[email protected]>

ckunki previously approved these changes Sep 23, 2024

View reviewed changes

ahsimb dismissed ckunki’s stale review via f442c8c September 23, 2024 11:09

ahsimb added 3 commits September 23, 2024 12:10

#58 Made two versions of the deploy_slc fixture

f442c8c

#58 Made two versions of the deploy_slc fixture

fee95ac

#58 Made two versions of the deploy_slc fixture

6c8238e

ckunki previously approved these changes Sep 23, 2024

View reviewed changes

ahsimb dismissed ckunki’s stale review via a3ec49a September 23, 2024 14:13

#58 Applied the suggestion from the review.

a3ec49a

ckunki approved these changes Sep 23, 2024

View reviewed changes

ahsimb merged commit d1dfad9 into main Sep 23, 2024
1 check passed

ahsimb deleted the bug/58-pytest-slc-dependency branch September 23, 2024 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#58 Made the pytest-slc plugin using the pyexasol_connection #59

#58 Made the pytest-slc plugin using the pyexasol_connection #59

ahsimb commented Sep 20, 2024 •

edited

Loading

tkilias Sep 22, 2024

ckunki Sep 23, 2024

ckunki Sep 23, 2024

tkilias Sep 23, 2024

ahsimb Sep 23, 2024

tkilias Sep 23, 2024

ckunki Sep 23, 2024

tkilias Sep 23, 2024

ckunki Sep 23, 2024

ckunki Sep 23, 2024

ahsimb Sep 23, 2024

ckunki Sep 23, 2024

ckunki Sep 23, 2024

	f'CREATE OR REPLACE {LANGUAGE_ALIAS} SCALAR SCRIPT "{{schema}}"."{{udf_name}}"() '
	f'CREATE OR REPLACE {LANGUAGE_ALIAS} SCALAR SCRIPT "{schema}"."{udf_name}"() '

		Here, we override the default `language_alias` fixture providing a "meaningful" name. The language container will be
		activated with this language alias. Note, that by default the test will run twice - once for each backend.

-Here, we override the default `language_alias` fixture providing a "meaningful" name. The language container will be
-activated with this language alias. Note, that by default the test will run twice - once for each backend.
+The example test case overrides two fixtures that are used by fixture `deployed_slc` from this plugin:
+* Fixture `language_alias` to provide a meaningful name.
+* and fixture `slc_builder` to define the SLC to be used in the example test case.
+The language container will be activated with value returned by fixture `language_alias`.
+Note, that by default the test will run twice - once for each backend.

#58 Made the pytest-slc plugin using the pyexasol_connection #59

#58 Made the pytest-slc plugin using the pyexasol_connection #59

Conversation

ahsimb commented Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahsimb commented Sep 20, 2024 •

edited

Loading