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

GitLab and git-lfs enhance #213

Closed
wants to merge 10 commits into from
Closed

GitLab and git-lfs enhance #213

wants to merge 10 commits into from

Conversation

icode
Copy link
Contributor

@icode icode commented Nov 10, 2018

  • [GitLab] Support hashed store path
  • Support remote git-lfs http server

close #212

@@ -24,6 +24,7 @@
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public class GitLabHookEvent {
private static final ObjectMapper mapper = new ObjectMapper();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it thread-safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course!

Mapper instances are fully thread-safe provided that ALL configuration of the instance occurs before ANY read or write calls. If configuration of a mapper is modified after first usage, changes may or may not take effect, and configuration calls themselves may fail. If you need to use different configuration, you have two main possibilities:
Construct and use ObjectReader for reading, ObjectWriter for writing. Both types are fully immutable and you can freely create new instances with different configuration using either factory methods of ObjectMapper, or readers/writers themselves. Construction of new ObjectReaders and ObjectWriters is a very light-weight operation so it is usually appropriate to create these on per-call basis, as needed, for configuring things like optional indentation of JSON.
If the specific kind of configurability is not available via ObjectReader and ObjectWriter, you may need to use multiple ObjectMapper instead (for example: you can not change mix-in annotations on-the-fly; or, set of custom (de)serializers). To help with this usage, you may want to use method copy() which creates a clone of the mapper with specific configuration, and allows configuration of the copied instance before it gets used. Note that copy() operation is as expensive as constructing a new ObjectMapper instance: if possible, you should still pool and reuse mappers if you intend to use them for multiple operations.

@@ -33,6 +33,8 @@
* @author Artem V. Navrotskiy <bozaro@users.noreply.github.com>
*/
public final class SessionContext {
static final ThreadLocal<SessionContext> LOCAL = new ThreadLocal<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why you need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get current socket user and repo info at any where in socket thread.
All lfs read interface not have User parameter, and repo info, url path mapper file path

@slonopotamus
Copy link
Collaborator

Tests do not compile: https://travis-ci.org/bozaro/git-as-svn/jobs/453166346#L694

/home/travis/build/bozaro/git-as-svn/src/test/java/svnserver/ext/gitlfs/storage/network/LfsHttpStorageTest.java:87: error: method getReader in class LfsHttpStorage cannot be applied to given types;

@icode
Copy link
Contributor Author

icode commented Nov 12, 2018

Tests do not compile: https://travis-ci.org/bozaro/git-as-svn/jobs/453166346#L694

/home/travis/build/bozaro/git-as-svn/src/test/java/svnserver/ext/gitlfs/storage/network/LfsHttpStorageTest.java:87: error: method getReader in class LfsHttpStorage cannot be applied to given types;

I change it

@icode
Copy link
Contributor Author

icode commented Dec 4, 2018

All checks have passed, please merge it!

@icode
Copy link
Contributor Author

icode commented Dec 6, 2018

@slonopotamus please merge it!

@slonopotamus
Copy link
Collaborator

Hi, @icode. I'm currently working on a modified version of your PR. Most important change that I want to do is to remove threadLocal and instead access gitlab lfs not on behalf of connected user but on behalf of gitlab token that you specify in git-as-svn config. ETA: this or next week.

@icode
Copy link
Contributor Author

icode commented Dec 7, 2018

@slonopotamus
Copy link
Collaborator

Yeah, I do understand that. ThreadLocal approach means git-as-svn doesn't have objective picture of repository contents, instead he has a subjective view depending on current user. This is dangerous and can break various places where we cache stuff regardless on current user in the future.

You don't need to rebase your PR, I'll handle that.

slonopotamus pushed a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#212 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#212 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#212 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#212 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#212 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
@slonopotamus
Copy link
Collaborator

I'm not going to take this PR due to my concerns about thread local and the fact that we lose objective view on repository contents that can be dangerous to different caches git-as-svn uses. Instead, see #222.

slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 8, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
slonopotamus added a commit to slonopotamus/git-as-svn that referenced this pull request Apr 9, 2019
Fixes git-as-svn#175, git-as-svn#212, git-as-svn#213.

This commit is based on code from git-as-svn#213 by icode.

Co-authored-by: icode <intelligentcodemail@gmail.com>
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.

How to use object store for lfs
2 participants