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

#138: Add extension #139

Merged
merged 22 commits into from
Sep 7, 2023
Merged

#138: Add extension #139

merged 22 commits into from
Sep 7, 2023

Conversation

kaklakariada
Copy link
Collaborator

Closes #138

extension/generate-description.sh Outdated Show resolved Hide resolved
extension/src/install.ts Outdated Show resolved Hide resolved
extension/src/extension.test.ts Outdated Show resolved Hide resolved
extension/src/test-utils.ts Show resolved Hide resolved
extension/src/uninstall.ts Outdated Show resolved Hide resolved
src/test/java/com/exasol/rls/extension/ExtensionIT.java Outdated Show resolved Hide resolved
src/test/java/com/exasol/rls/extension/ExtensionIT.java Outdated Show resolved Hide resolved
src/test/java/com/exasol/rls/extension/ExtensionIT.java Outdated Show resolved Hide resolved
src/test/java/com/exasol/rls/extension/ExtensionIT.java Outdated Show resolved Hide resolved
@ckunki ckunki merged commit 06bebd0 into main Sep 7, 2023
4 checks passed
@ckunki ckunki deleted the kaklakariada/issue138 branch September 7, 2023 12:55
Copy link

Choose a reason for hiding this comment

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

You might want to rewrite this at some point in the future in a less error-prone language.

Copy link

Choose a reason for hiding this comment

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

Options Typescript, Java or a Python script without dependencies (Python is usually somehow available and without dependencies, you are good to go.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Next time I touch this file I will think about refactoring this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had similar headaches 🙂

@@ -0,0 +1,31 @@
{
"name": "row-level-security-extension",
"version": "0.0.0",
Copy link

Choose a reason for hiding this comment

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

0.0.0 as a version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's fine because we will never publish this to NPM. Duplicating the version number here is just error prone and easy to forget.

)`;

export function buildCreateScriptCommand(qualifiedScriptName: string, luaScriptContent: string) {
return `CREATE OR REPLACE LUA ADAPTER SCRIPT ${qualifiedScriptName} AS
Copy link

Choose a reason for hiding this comment

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

oh, you are constructing the script here, hm, interesting we are doing this already in python. Ok, also a thing, I need to keep in mind.


export function getInstalledExtension(): any {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
return (global as any).installedExtension
Copy link

Choose a reason for hiding this comment

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

where is global from?

Copy link
Collaborator Author

@kaklakariada kaklakariada Sep 8, 2023

Choose a reason for hiding this comment

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

This is the "global" JS object. We use it to register an extension to make it available to the Extension Manager's JS runtime, see Go function loadExtension().

@@ -166,6 +165,59 @@
</arguments>
</configuration>
</execution>
<!-- Build extension -->
<execution>
Copy link

Choose a reason for hiding this comment

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

Interesting, so you actually already call npm from maven, so probably could also call a typescript script which inlines the script

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually we call the shell script during npm run build (see package.json) ;)

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.

Add support for extension manager
3 participants