Skip to content

ST6RI-682 Implemented loading of models from repository #616

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

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

TheKorpos
Copy link
Contributor

@TheKorpos TheKorpos commented Feb 4, 2025

This PR adds basic model download capability using the standard API.

Overview

The model download functionality can be used

  1. In Jupyter using magic commands
  2. In Eclipse IDE as a 'right click' action on projects
  3. As CLI java application

In Jupyter, the downloaded models are stored in memory and can be used throughout the work session. The Eclipse and CLI versions serialize the model into separate XMI files (.sysmlx). In Eclipse IDE .kerml and .sysml files are now added to the Xtext index so their content can be referenced using the textual syntax.

Common changes

  • .sysmlx and .kermlx parsing, indexing:
    • SysMLxRuntimeModule, SysMLxStandaloneSetup has been added
    • KerMLxRuntimeModule, KerMLxStandaloneSetup has been added
  • APIs for model download, JSON to EMF transformation
    • ProjectRepository: repository access, download
    • RepositoryContentFetcher: JSON to EMF transformation, XMI serialization (.sysmlx)

Jupyter changes

  • new magic commands have been added
    • %projects [-h][-help]
      Lists all projects names and their ids
    • %load <project name> [-h][-help]
      downloads the project and adds its contents to the xtext index.
    • %help has been updated
    • help has been added to new commands

Eclipse changes

  • SysMLv2 menu has been added to the popup menu of projects
    • Generate Library Index has been moved into it
  • Registered .sysmlx and .kermlx
  • .sysmlx and .kermlx resources are added to the xtext index
  • Registered simple Ecore editor for the extensions
  • References in text opens the Ecore editor and jumps to the element
  • "Pull model from API" right click action has been added:
    Downloads and saves models in .sysmlx
    • Prerequisites:
      .settings/org.omg.sysml.remote.properties exists with the following contents:
base.url=repository url
remote.projectId=project id

NOTE: the action shows up regardless for now. The file is needed to perform the operation.
NOTE: a specific tester was implemented to hide the action from projects without the required properties file. However, this tester is not called before the sysml.ui plugin is activated, e.g. before a sysml editor is opened.

CLI app

  • org.omg.sysml.xtext.util.SysMLRepositoryLoadUtil has been added
    • arguments:
      -p, -project: name of the project in the repository
      -l, -library: location of the standard library
      -t, -target: target location for saved resources
      -b, -base (optional): URL of the repository, default: http://localhost:9000

ujhelyiz and others added 3 commits February 4, 2025 16:52
 - Introduces a new SysMLv2 menu for all contributions
 - Renames repository upload feature
 - Command framework should check for the existence of repository
descriptor for showing the command
Copy link
Member

@seidewitz seidewitz left a comment

Choose a reason for hiding this comment

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

  • Instead of %publications and %download use %projects and %load.
  • The %load command (and utility) should have an option to use the project ID instead of the name.

 - changed %publications to %projects
 - changed %download to %load

 - added optional arguments to %load
 	--id=<ID>
 	--name=<NAME>
 - changed SysMLRepositoryLoad util arguments
 	-n, -name to reference project by name
 	-id to reference project by id
@TheKorpos
Copy link
Contributor Author

TheKorpos commented Feb 5, 2025

  • Instead of %publications and %download use %projects and %load.
  • The %load command (and utility) should have an option to use the project ID instead of the name.

@seidewitz
I've changed the magic command names as you requested and added a load by id option.
The magic command %load works like this now
%load --id=<PROJECT ID> loads the model by id
%load --name=<PROJECT NAME>, %load <PROJECT NAME> loads the model by name

as for the utility project name is now passed with -n <PROJECT NAME> or -name <PROJECT NAME>.
alternatively -id <PROJECT ID> can be used

@hpdekoning
Copy link
Member

@TheKorpos
Hi Laszlo, just happened to scan through this PR, and have a minor comment w.r.t to long-names for options.

org.omg.sysml.xtext.util.SysMLRepositoryLoadUtil has been added
arguments:
-p, -project: name of the project in the repository
-l, -library: location of the standard library
-t, -target: target location for saved resources
-b, -base (optional): URL of the repository, default: http://localhost:9000/
as for the utility project name is now passed with -n or -name .
alternatively -id can be used

In my experience, most open source tools use the GNU convention for command line options is to use single hyphen like -n for single letter (short-name) options and double hyphen like --name for fully spelled out long-name options (two or more characters), which would also be in line with the magic commands. From documentation https://commons.apache.org/proper/commons-cli/ the org.apache.commons.cli package does support this. Or perhaps there is another reason not to stay with GNU conventions?

@TheKorpos
Copy link
Contributor Author

@TheKorpos Hi Laszlo, just happened to scan through this PR, and have a minor comment w.r.t to long-names for options.

org.omg.sysml.xtext.util.SysMLRepositoryLoadUtil has been added
arguments:
-p, -project: name of the project in the repository
-l, -library: location of the standard library
-t, -target: target location for saved resources
-b, -base (optional): URL of the repository, default: http://localhost:9000/
as for the utility project name is now passed with -n or -name .
alternatively -id can be used

In my experience, most open source tools use the GNU convention for command line options is to use single hyphen like -n for single letter (short-name) options and double hyphen like --name for fully spelled out long-name options (two or more characters), which would also be in line with the magic commands. From documentation https://commons.apache.org/proper/commons-cli/ the org.apache.commons.cli package does support this. Or perhaps there is another reason not to stay with GNU conventions?

@hpdekoning
Thanks for the pointing this out!

org.apache.commons.cli does have a GNUParser but it's marked deprecated. I used the suggested DefaultParser instead. This parser allows long-name options with both single and double hyphen. Intrestigely the dedicated GNUParser allows this behavior as well it seems.

There is a allowPartialMatching mode too. This is true by default. This allows partially spelled long-names as long as there is only one matching option. I think I should turn this off as it makes --n=something a valid option.

I'm not sure that allowing single hyphen long-names is a huge issue as long as it also accepts the GNU argument syntax.

@seidewitz seidewitz self-assigned this Mar 27, 2025
@seidewitz seidewitz marked this pull request as draft March 27, 2025 20:37
Copy link
Contributor

@ujhelyiz ujhelyiz left a comment

Choose a reason for hiding this comment

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

In general this looks fine, but it is possible that some comment added to #633 might be better handled here.

@seidewitz
Copy link
Member

I'm not sure that allowing single hyphen long-names is a huge issue as long as it also accepts the GNU argument syntax.

Allowing single-hyphen long-name options is OK, as long as double-hyphen is allowed, too. But the usage text should show them using double-hyphen, because that is the generally expected syntax.

Copy link
Member

@seidewitz seidewitz left a comment

Choose a reason for hiding this comment

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

  • Update the output from the %projects command so it has the following format:
    API base path: <base path URL>
    
    Project <project name> (<project UUID>)
    ...
    
  • The %load command should provide clearer feedback on intermediate progress.
  • The returned success result from the %load command should have the form:
    Loaded Project <project name> (<project UUID>)
    
  • I question whether we should include the --branch option as "experimental". Either we should include it and tests, with whatever limitations we currently have, or we should just not include it in this release.

@seidewitz seidewitz marked this pull request as ready for review May 1, 2025 16:55
@TheKorpos
Copy link
Contributor Author

@seidewitz

I changed the output of %projects as requested.

  • The %load command should provide clearer feedback on intermediate progress.
  • The returned success result from the %load command should have the form:
    Loaded Project <project name> (<project UUID>)
    

I changed the feedback. Now it only displays the two long running processes: the library UUID collection and the download/model creation. The UUID collection only happens on the first load so it's only displayed once when it happens.

This is how the output looks:

Caching library UUIDs...
Downloading model...
Loaded Project <project name> (project UUID)
  • I question whether we should include the --branch option as "experimental". Either we should include it and tests, with whatever limitations we currently have, or we should just not include it in this release.

The branch upload/download works with the pilot implementation but I'm not sure how much value it adds with the current limitations. Currently uploading to a branch (other than main) is basically the same as uploading to a seperate project.

@seidewitz seidewitz added this to the 2025-04 milestone May 6, 2025
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.

4 participants