Skip to content

Rewrite code for language services to use protobufjs library and remo…#123

Merged
igormkdd merged 7 commits intomasterfrom
feature/IAC-701-vscode-vro-javascript-autocomplete
Apr 10, 2023
Merged

Rewrite code for language services to use protobufjs library and remo…#123
igormkdd merged 7 commits intomasterfrom
feature/IAC-701-vscode-vro-javascript-autocomplete

Conversation

@igormkdd
Copy link
Copy Markdown
Contributor

@igormkdd igormkdd commented Mar 7, 2023

Description

  • Rewrite code for language services to use protobufjs library and remove "mvn collect" plugin dependency
  • Fix startup issues and fix minor bugs
  • Update autocompletion to work without the plugin, and to directly get the data from vRO

Checklist

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have tested against live vRO/vRA, if applicable
  • My changes have been rebased and squashed to the minimal number of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixed #XXX - or Closed #XXX - prefix to auto-close the issue

Testing

Release Notes

Screenshot

Related issues and PRs

…ve plugin dependency + fix startup and minor bugs + update autocompletion

Signed-off-by: Igor Janevski <ijanevski@vmware.com>
@igormkdd igormkdd requested a review from a team as a code owner March 7, 2023 14:41
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 7, 2023

Codecov Report

Merging #123 (8330e4d) into master (1560082) will increase coverage by 4.21%.
The diff coverage is 61.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   50.00%   54.21%   +4.21%     
==========================================
  Files          40       40              
  Lines         790      806      +16     
  Branches      116      117       +1     
==========================================
+ Hits          395      437      +42     
+ Misses        356      331      -25     
+ Partials       39       38       -1     

@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Signed-off-by: Igor Janevski <ijanevski@vmware.com>
@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Copy link
Copy Markdown
Contributor

@ivo-kotev ivo-kotev left a comment

Choose a reason for hiding this comment

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

Please check the comments.
We will need for sure some MD file that can explain how the collection is working so this can be supported later on.

Vra = "com.vmware.pscoe.vra:vra-package",
VraNg = "com.vmware.pscoe.vra-ng:vra-ng-package"
VraNg = "com.vmware.pscoe.vra-ng:vra-ng-package",
Polyglot = "com.vmware.pscoe.polyglot:polyglot-project"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we are listing all supported, we are missing vrops, vcd, vrli - basically all supported are defined here: https://github.com/vmware/build-tools-for-vmware-aria/tree/main/maven/base-package

const lineContent = document.getLineContentUntil(position)

this.logger.debug(`Trying to provide auto completion for line '${lineContent}'`)
this.logger.info(`Trying to provide auto completion for line '${lineContent}'`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to have this logged for all users?

this.getModulesForCollection(workspaceFolder, undefined, modules)
if (modules.length === 0) {
this.logger.debug("Skipping workspace collection as there are no JS action modules in the project")
this.logger.info("Skipping workspace collection as there are no JS action modules in the project")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need this level of logging in Info mode?

export class WorkspaceCollection {
private readonly logger = Logger.get("WorkspaceCollection")
private readonly debounceTriggerCollection = _.debounce(this.triggerCollectionAndRefresh, 10000)
private readonly debounceTriggerCollection = _.debounce(this.triggerCollectionAndRefresh, 1000)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we move these timeout and sleep configurations to a constants file?

returnType?: string
}

export interface HintPlugin {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this about the vRDT version? or for backward compatibility?

}
})
.catch(error => {
this.logger.info("Could not connect to vRO. Collecting offline hints collection...")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does it mean "Collecting offline hints collection..." it's going to use the data that is already cached locally? If yes - maybe rephrase a bit the message?

if (fs.existsSync(vpkFilePath)) {
fs.unlinkSync(vpkFilePath)
}
const parsedLink = link[0].substring(9).toString()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we expect cases with less links? count=0? And maybe add a comment what is specific for this link0?

const command = `mvn o11n-hint:collect -DoutputHintDirectory="${outputDir}" -pl ${modules.join(",")} -e`
const cmdOptions = { cwd: workspaceFolder.uri.fsPath }
const fullPath = path.join(workspaceFolder.uri.fsPath, modules.join(","))
const modulesPath = path.join(fullPath, "src/main/resources")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this should work for both JS and TS projects, the path for TS projects is only src/?

metadata: {
timestamp: Date.now(),
serverName: "",
serverVersion: "0.0.1",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it going to be useful to cache the server name and version as well? Or we are not using this information?

- **`packages/node/vro-language-server`** - A Node.js implementation of the [Language Server Protocol (LSP)](https://github.com/Microsoft/language-server-protocol). It analyses type information received from the vRO server and maintains semantic knowledge about a vRO solution implemented in JavaScript. Depends on vRO Hint Plugin that is part of the [vRealize Build Tools](https://flings.vmware.com/vrealize-build-tools) fling. _No longer maintained, still remains for compatibility._
- **`packages/node/vro-language-server/protocol`** - A set of [Protocol Buffer](https://developers.google.com/protocol-buffers/) message definitions that are used by the vRO language server and the vRO Hint plug-in as a serialization format for communication and storage purposes. _No longer maintained, still remains for compatibility._
- **`packages/node/vro-language-server`** - A Node.js implementation of the [Language Server Protocol (LSP)](https://github.com/Microsoft/language-server-protocol). It analyses type information received from the vRO server and maintains semantic knowledge about a vRO solution implemented in JavaScript. The updated version is operational with vRO 8, but not with the vRO 7 anymore. The plugin is not needed for fetching the data from the vRO, but rather only a connection to it is required and a valid maven profile. Moreover, local js files are also analyzed.
- **`packages/node/vro-language-server/protocol`** - A set of [Protocol Buffer](https://developers.google.com/protocol-buffers/) message definitions that are used by the vRO language server as a serialization format for communication and storage purposes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to add a bit more documentation on how this is working, maybe a separate md file that explains the communication flow and the components/files involved in this communication.

Also - is this going to work for vRO servers that are using vRA authentication? I think currently all operations for vRDT support only basic auth, which makes most of the things unusable in vra 8.8+ without additional vro configuration?

@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

…-by: Igor Janevski <ijanevski@vmware.com>

Signed-off-by: Igor Janevski <ijanevski@vmware.com>
@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

…evski <ijanevski@vmware.com>

Signed-off-by: Igor Janevski <ijanevski@vmware.com>
@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@vmwclabot
Copy link
Copy Markdown

@igormkdd, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

@igormkdd igormkdd merged commit 0b7cf16 into master Apr 10, 2023
@igormkdd igormkdd deleted the feature/IAC-701-vscode-vro-javascript-autocomplete branch April 10, 2023 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants