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

Implement Project service #438

Merged
merged 10 commits into from
Dec 8, 2023
Merged

Implement Project service #438

merged 10 commits into from
Dec 8, 2023

Conversation

adambabik
Copy link
Collaborator

This PR implements a new service to load tasks from a project.

Usage

After starting the server using runme server, you will be able to list the new ProjectService:

$ grpcurl -plaintext -unix /tmp/runme.sock list runme.project.v1.ProjectService
runme.project.v1.ProjectService.Load

There is only one method called Load and it streams a list of events while loading a file- or dir-based project.

For a file-based project:

$ grpcurl -plaintext -d "{\"file\":{\"path\":\"/path/to/README.md\"}}" -unix /tmp/runme.sock runme.project.v1.ProjectService/Load
{
  "type": "LOAD_EVENT_TYPE_STARTED_WALK",
  "startedWalk": {

  }
}
{
  "type": "LOAD_EVENT_TYPE_FOUND_FILE",
  "foundFile": {
    "filepathAbs": "/path/to/README.md"
  }
}
{
  "type": "LOAD_EVENT_TYPE_FINISHED_WALK",
  "finishedWalk": {

  }
}
{
  "type": "LOAD_EVENT_TYPE_STARTED_PARSING_DOC",
  "startedParsingDoc": {
    "filepathAbs": "/path/to/README.md"
  }
}
...

For a dir-based project:

$ grpcurl -plaintext -d "{\"directory\":{\"path\":\"/path/to/project\",\"respect_gitignore\":true}}" -unix /tmp/runme.sock runme.project.v1.ProjectService/Load

@adambabik adambabik mentioned this pull request Nov 25, 2023
@adambabik adambabik marked this pull request as ready for review November 25, 2023 20:51
@sourishkrout
Copy link
Member

Got it working locally: https://app.runme.dev/cell/clphggfii0013s601ou0tpnrk

@@ -0,0 +1 @@
x��Qj1 D��S述Ȋ�� ��7�-;Y����ܿ.������m߷��i�R�%�#���ѳ�D�nEĭj�d�T���^�8��gr.̩e�Ps���L>eҚ�y�{��÷��Y�_�j9�۳�>�c�m��m�sd�@�hf;EGy���#���r��|k��_YxL�
Copy link
Member

Choose a reason for hiding this comment

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

@adambabik what's all this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to have a real git repository to do integration tests. As it's possible to embed one git repository in another, except for git submodule but then the submodule must be a standalone repo, I just changed .git to .git.bkp and added it directly. Before running the tests, I do cp -r .git.bkp .git.

There are a few nicer alternatives:

  • Have it as an actual standalone repo and use submodules.
  • Compress and commit a single tar file.
  • Create a repo from scratch as a test setup step in the temp dir.

I think the last one should be doable purely using go-git. I will try to do that.

Copy link
Member

Choose a reason for hiding this comment

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

All good. If it works, it works. Mainly wanted to make sure it's intentional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I will fix it in another PR.

@sourishkrout
Copy link
Member

sourishkrout commented Nov 27, 2023

Added examples here #442, please consider merging @adambabik.

@adambabik
Copy link
Collaborator Author

Got it working locally: https://app.runme.dev/cell/clphggfii0013s601ou0tpnrk

One thing that I believe would be good to change is to reverse some params like respect_gitignore. Currently, if this param is not provided, .gitignore is not respected. I will change it to skip_gitignore or similar, so that the default behavior is more friendly.

@adambabik adambabik force-pushed the adamb/projects-service branch from 7be5dc5 to be8f461 Compare November 30, 2023 18:43
@sourishkrout
Copy link
Member

When you have a moment, please merge/rebase the latest main into this branch, @adambabik.

adambabik and others added 6 commits December 6, 2023 19:29
@adambabik adambabik force-pushed the adamb/projects-service branch from 94e177d to 302d72f Compare December 6, 2023 18:30
@sourishkrout
Copy link
Member

Based on starting with a VS Code Task-based integration here are some notes:

  1. Currently the task will be "packaged" solely based on the API's data provided at "listing time". At "execution time" the deserializer is being run and the cell is matched based on ID to load all associated attributes etc for execution. This laziness is nice because it avoids costly deserialization of all tasks/notebooks.
  2. The main cosmetic feature I would want before merging is the notion of named = yes/no per task where the yes is only present for tasks that were explicitly named and not auto-generated. This allows the UI to be more nuanced, just like TUI (and the u shortcut key toggle).

@adambabik
Copy link
Collaborator Author

@sourishkrout I addressed (2) in 5a4dc61.

Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@sourishkrout sourishkrout merged commit 1065b4e into main Dec 8, 2023
4 checks passed
@sourishkrout sourishkrout deleted the adamb/projects-service branch December 8, 2023 20:16
@sourishkrout sourishkrout mentioned this pull request Jan 3, 2024
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