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

Improve communication between MAP-Client and PMR #65

Merged
merged 5 commits into from
Sep 20, 2023

Conversation

tsalemink
Copy link
Contributor

This PR addresses issues occurring with the communication between the MAP-Client and the Physiome Model Repository (PMR).

The usage in the MAP-Client of a couple pmr2.wfctrl classes has been updated to align with the current PMR2 API:

  • The PMR2 CmdWorkspace class requires a Cmd object instance as an argument. The get_cmd_by_name method we are using in the MAP-Client returns a class type so the return value must be instantiated before passing it to the CmdWorkspace constructor.
  • All PMR2 BaseDvcsCmd subclasses now output the Git command return-code in addition to the stdout and stderr streams. This can be used to more accurately filter error messages passed to the MAP-Client from the pmr2.wfctrl package. This is now being used in the MAP-Client to address an issue we have been having where some of the Git push commands we are executing to submit content to PMR return error messages even when the return_code shows that the push was successful.

Two methods have been added to the WorkflowStepMountPoint: gitInclude and createGitIgnore.

  • The gitInclude method can be overridden by a plugin implementation to specify a list of files or file types that should be included under the step's workflow sub-directory in the version control of the workflow.
  • The createGitIgnore method is called by the MAP-Client to create a .gitignore file for each of the workflow steps in the workflow directory. This will ignore all files in the step's workflow sub-directory except those specified by the plugin's gitInclude method. The WorkflowWidget._commitChanges method has been updated slightly so that step workflow sub-directories are included in the commit to PMR. Note that since the plugin specific .gitignore files exclude all file types by default, the step workflow directory will only be committed to PMR if the plugin implements the gitInclude method.

tsalemink added 4 commits June 1, 2023 20:10
The CmdWorkspace class requires a cmd object instance as an argument. The get_cmd_by_name method returns a class type so the return value must be instantiated before passing it to the CmdWorkspace constructor.
All PMR2 BaseDvcsCmd subclasses now output the Git command return-code in addition to the stdout and stderr streams. This can be used to more accurately filter error messages passed to the MAP-Client from the pmr2.wfctrl package.

A return code check has been added to the PMRTool.pushToRemote method to ensure that the stderr stream is only logged as an error if the return code is non-zero.
This commit adds two methods to the WorkflowStepMountPoint: gitInclude and createGitIgnore. These methods allow a plugin implementation to specify what files in the step's workflow sub-directory should be included in version control.

The gitInclude method can be overridden by a plugin implementation to specify a list of files or file types that should be included in the version control of the workflow.

The createGitIgnore method is called by the MAP-Client to create a .gitignore file for each of the workflow steps in the workflow directory. This will ignore all files in the step's workflow sub-directory except those specified by the plugin's gitInclude method.
This updates the way the step specific .gitignore files are created to allow the user to override them with a single .gitignore file in the workflow root directory.
@tsalemink
Copy link
Contributor Author

I thought I should include some additional information about the limitations of the workflow step .gitignore files.

I have chosen to use plugin specific .gitignore files to specify what should be added to workflow version control because this works well with the existing PMR Git communication but will also work if the workflow is being manually version controlled (e.g., cloned from a mapclient-workflows repository). Also, it only requires a minimal implementation to get this to work. Unfortunately, this makes it somewhat difficult to allow the user to override the .gitignore files for the workflow steps since the sub-directory .gitignore files will automatically override the one in the workflow root directory. Because of the default .gitignore hierarchy I am currently removing all plugin specific .gitignore files if there is a .gitignore in the workflow root directory to give the user this override ability. This works for both the internal PMR workflow version control and external workflow version control. It is possible to implement a more complex override method that takes the contents of all .gitignore files into consideration rather than just deleting the plugin specific files, but would require some significantly more complex pattern matching so I have skipped that for now.

Alternatively, we could replace the plugin specific .gitignore files with something custom - like a .gitinclude file. This would give us more control over the Git communication between the MAP-Client and PMR internally, but we would lose the ability to have external workflow version control make use of these files.

@hsorby hsorby merged commit d7d8092 into MusculoskeletalAtlasProject:main Sep 20, 2023
@tsalemink tsalemink deleted the pmr2 branch September 20, 2023 23:59
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.

2 participants