-
Notifications
You must be signed in to change notification settings - Fork 738
Include support for git-remote-s3 #6260
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
✅ Deploy Preview for nextflow-docs-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
I have added a prototype implementation of a push command to enable to push a local directory to a remote repository. It uses the jgit, so in theory, it should work with whatever git repository but I have just tested it with s3-git-remote.
Two main use cases: 1- The specified folder is not a git repository: After the push a user can run 2- The folder is already a git repo: If the user has not specified anything, it uses the remote and revision currently set in the git folder. |
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Is the command with the repo option in the example correct? I'd have expected
instead of
also because later it shows the one without repo
|
In the current implementation, the main argument is the path that we want to push. I could do as you propose assuming that the path to push is the current working dir As commented before, there are two cases: when the folder is already a local git repo (contains .git) or not. The one without repo is when .git already exists and you want to update with current changes. It will push to the repository in origin and current branch. |
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@claude review |
Claude finished @jorgee's task —— View job Code Review: S3 Git Remote ImplementationReviewing the S3 git-remote implementation for Nextflow. This is a substantial addition with 21 files changed. Todo List:
Review SummaryThis is an impressive implementation that adds S3 git-remote support to Nextflow. The implementation follows good architectural patterns and integrates well with the existing codebase. Overall, this is solid work with good test coverage and comprehensive functionality. Architecture & Design ✅Strengths:
Design Decisions:
JGit Transport Implementation 🔍S3BaseConnection.java (
S3FetchConnection.java:
S3PushConnection.java:
Nextflow Integration Layer ✅S3ProviderConfig.groovy:
S3RepositoryProvider.groovy:
CmdPush Command 📝Comprehensive Implementation:
Potential Issues:
Test Coverage 🧪Unit Tests:
Integration Tests:
Security & Error Handling 🔒Security:
Error Handling:
Specific RecommendationsCritical Issues to Address:
Minor Improvements:
Documentation & Examples ✅
Performance Considerations
ConclusionThis is a well-architected and comprehensive implementation. The code follows Nextflow patterns, has good test coverage, and provides a complete S3 git-remote solution. The two main issues (reflection usage and inefficient file reading) should be addressed, but they don't block the core functionality. Recommendation: Approve with minor fixes. This significantly enhances Nextflow's git repository support capabilities. |
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Updates:
|
S3 can be used to store git repositories using the git-remote-s3 implementation of remote helper.
This branch provides support for this S3 repositories as part of the
nf-amazon
plugin.It implements a JGit
Transport
with the Fetch and Push connection as well as theRepositoryProvider
,ProviderConfig
RepositoryFactory
required to support s3 git repositories.The fetch connection is used to clone and pull branches of the repository in the clone, pull and run commands
The push connection is able to push branches.
Current Limitations:
Due to a limitation in the access rights of some JGit transport classes, when a branch is pushed it is correctly stored in the remote repository but the local repo refs are not updated with the reference to the remote ref (origin/branch)TODO: