Skip to content

cmd/gitbase: load siva files and git indistinctly #416

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

Merged
merged 6 commits into from
Aug 10, 2018

Conversation

mcarmonaa
Copy link
Contributor

Closes #414

The new usage looks like:

Usage:
  gitbase [OPTIONS] server [server-OPTIONS]

Starts a gitbase server instance

By default when gitbase encounters an error in a repository it
stops the query. With GITBASE_SKIP_GIT_ERRORS variable it won't
complain and just skip those rows or repositories.

Help Options:
  -h, --help             Show this help message

[server command options]
      -v                 Activates the verbose mode
      -d, --directories= Path where the git repositories are located (standard and siva), multiple directories can be defined. Accepts globs.
          --depth=       load repositories looking at less than <depth> nested subdirectories. (default: 1000)
          --no-git       disable the load of git standard repositories.
          --no-siva      disable the load of siva files.
          --host=        Host where the server is going to listen (default: localhost)
      -p, --port=        Port where the server is going to listen (default: 3306)
      -u, --user=        User name used for connection (default: root)
      -P, --password=    Password used for connection
          --pilosa=      URL to your pilosa server (default: http://localhost:10101) [$PILOSA_ENDPOINT]
      -i, --index=       Directory where the gitbase indexes information will be persisted. (default: /var/lib/gitbase/index) [$GITBASE_INDEX_DIR]
          --no-squash    Disables the table squashing.
          --trace        Enables jaeger tracing [$GITBASE_TRACE]
      -r, --readonly     Only allow read queries. This disables creating and deleting indexes as well. [$GITBASE_READONLY]

Changes

  • the flag to specify a list of directories now is -d, --directories so there's no more --git or --siva

gitbase server -d=/d1,/d2

  • The directories are traversed recursively, but with the new flag --depth you can specify how many levels from the specified directory must be checked.

gitbase server -d=/d1,/d2 --depth 1

  • Now siva files and git repositories are loaded even if the they are mixed so if you don't want to load some of them you can use the flags --no-git and --no-siva:

gitbase server -d=/d1,/d2 --no-siva

So if we have a directory structure:

.
├── a.siva
├── b.siva
├── repo-a
└── some-dir
    ├── c.siva
    └── repo-b

gitbase server -d=d1 -> will load a.siva,b.siva,repo-a,repo-b, c.siva
gitbase server -d=d1 --no-git -> will load a.siva, b.siva, c.siva
gitbase server -d=d1 --no-siva-> will load repo-a, repo-b
gitbase server -d=d1 --depth=1 -> will load a.siva, b.siva, repo-a

All the variants combining --no-git and --no-siva flags with --depth are allowed and work too.

On the other hand, there is some useless code left in repository_pool.go about load directories, should I remove it?

@src-d/data-retrieval WDYT?

@mcarmonaa mcarmonaa requested a review from a team August 8, 2018 11:40
@@ -262,6 +276,7 @@ func (p *RepositoryPool) addSivaFile(root, path string, f os.FileInfo) {

if strings.HasSuffix(f.Name(), ".siva") {
path := filepath.Join(path, f.Name())
fmt.Println(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@@ -182,6 +183,7 @@ func (p *RepositoryPool) AddGit(path string) (string, error) {
// AddGitWithID checks if a git repository can be opened and adds it to the
// pool. ID should be specified.
func (p *RepositoryPool) AddGitWithID(id, path string) (string, error) {
fmt.Println(id, path)
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

}

return nil
for _, file := range files {
if file.Name() == ".git" {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about bare repositories?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@mcarmonaa mcarmonaa force-pushed the improvement/load-siva-git-depth branch from 449e541 to 4b2ead1 Compare August 8, 2018 13:03
Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
@mcarmonaa mcarmonaa force-pushed the improvement/load-siva-git-depth branch from 4b2ead1 to 9dcd907 Compare August 8, 2018 13:05
@eiso
Copy link
Member

eiso commented Aug 8, 2018

I love this! Great for user-friendliness.

Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

we should change documentation too @mcarmonaa

@mcarmonaa
Copy link
Contributor Author

@ajnavarro sure, I'll do it too. Other than that, do you think I should remove all the code related to load repositories from repository_pool.go? Or do we want to keep it there to expose it although we are not using it anymore?

Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
@mcarmonaa mcarmonaa changed the title [WIP] cmd/gitbase: load siva files and git indistinctly cmd/gitbase: load siva files and git indistinctly Aug 9, 2018
@mcarmonaa
Copy link
Contributor Author

  • Documentation has been updated
  • Dockerfile has been updated to use --directories flag
  • Some code has been removed because is not used anymore

require.Exactly(t, test.expected, files)
})
}
}

func TestPatternPrefixDepth(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you delete that test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the function was tested here is not needed now so I removed it.

// that is, has the .siva extension
func (p *RepositoryPool) AddSivaFile(path string) error {
file := filepath.Base(path)
if !strings.HasSuffix(file, ".siva") {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would separate the logic related to check if the path is from a siva file, and add a siva repository to the repository pool. Per example, as we have a isGitRepo function, we should have a isSivaFile function to check if we can add that file to the pool.

return err
}

if info.IsDir() {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could split these two cases to their own functions, and maybe then simplify some of the conditions with return early clauses.

Copy link
Contributor Author

@mcarmonaa mcarmonaa Aug 9, 2018

Choose a reason for hiding this comment

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

I've placed the logic related to add a git repository in its own method, but I can't see how to simplify the conditions

@mcarmonaa mcarmonaa force-pushed the improvement/load-siva-git-depth branch from 5e13a54 to ed911c2 Compare August 9, 2018 10:50
…sitoriy when is added

Signed-off-by: Manuel Carmona <manu.carmona90@gmail.com>
@mcarmonaa mcarmonaa force-pushed the improvement/load-siva-git-depth branch from ed911c2 to adb1156 Compare August 9, 2018 11:34
@ajnavarro ajnavarro merged commit 4d99164 into src-d:master Aug 10, 2018
@mcarmonaa mcarmonaa deleted the improvement/load-siva-git-depth branch August 10, 2018 11:11
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.

5 participants