#46 git import #78

evolverine · 2020-03-25T12:38:55Z

A few notes:

you can ignore the first four commits. I made them when I wasn't yet aware that Dulwich was already a dependency of this project, and I was including GitPython. This is reverted in d502c48
commit e72ef09 introduces a change in the directory structure that the repository is created in: instead of "CrowdAnki/repo_name/git" it uses "CrowdAnki/user_name/repo_name/". This is reverted in the last commits, but I do want to ask you about it, because I think it's much better to use the github username in the path, to avoid naming conflicts. What do you think, will this also impact on other parts of the add-on?
the feature we have now is that we clone a repository instead of downloading the zip (and pull if the repository exists). I have not yet implemented the remote addition, but I'm happy to do it next if the direction seems good.

Also, two questions:

Instead of the Anki profile directory, shouldn't we now use the directory in the plugin settings to clone the repository?
For everyone but the repository maintainers the initially-cloned repository will have to be converted into a fork if they want to contribute any changes. Maybe we can automate that within this add-on using hub?

…ory by default instead of downloading the repository zip file

…t in place in future GitPython updates for the imports to work

…username and repository name, to prevent name clashes

…n with author renders it safe to change

crowd_anki/utils/uuid.py

crowd_anki/github/github_importer.py

Stvad · 2020-03-29T23:45:18Z

crowd_anki/github/github_importer.py

-    def download_and_import(self, repo):
+    def download_and_import_git(self, github_repo):
+        repo_parts = github_repo.split("/")
+        repo_dir = Path(self.collection.media.dir()).joinpath("..", "CrowdAnkiGit", repo_parts[-1], "git")


For the directory I think the ideal option would be to clone things into the place where snapshots go (it's available in config). The format there is snapshot_dir/<anki profile name>/<deck name>

Deck name is a bit tricky - if you follow the convention of naming the repo with the CrowdAnki deck name like here https://github.com/Stvad/Software_Engineering__git then you can use that, but I suspect you don't (being not a fun of those names & all :p)

So another option can be is storing the snapshot destination on per deck basis. Something like:

If you're importing deck from git repo -> set snapshot destination to the original import directory

If it's not set -> set it to the default one

Use these names on snapshot

I'll admit that I didn't understand very well what you meant by the three bullet points above. Please see the commits I just pushed, and how they relate to what you suggest, thank you.

Have you had a chance to take a look? Thanks.

Should be able to take a closer look today/tomorrow.

What the comment above was getting at that if the name of repository does not match the name of the deck - then initial import would work but subsequent snapshots will go to a different directory which is problematic. I proposed a way to solve that by changing how we determine the snapshot directory for deck

But the name of the repository does not determine the name of the deck: when importing from github, the deck name is taken from deck.json So even if I import the "covid-19-anki-deck, the deck I see in Anki is "COVID-19" (in fact, I see "COVID-19_2", "COVID-19_3", etc, but right now that's beyond the point). Indeed, though, the name of the imported repository directory will be covid-19-anki-deck, so I see your point.

Nevertheless, the user can rename the deck to something wildly different, and they can also move the cards in and out of it at will. Being driven by the deck name will lead to a multitude of issues.

The way it would make the most sense for me for all this to happen is if each deck had a property called remote_url or something like that, which pointed to (either the original, or to the forked) remote repository. Then we'd use this property to determine the directory names for the repositories.
I'm not familiar enough with the Anki codebase to know whether that's something that could be implemented?

crowd_anki/github/github_importer.py

Stvad · 2020-03-30T00:07:43Z

Integrating with github to create forks/etc - In general seems like a good idea for simplifying the flow, we should discuss how interaction would look like before going there though. And should be a separate CR.

Also using the actual cli tool really makes life harder distribution-wise, I imagine there is python lib out there for talking to Github API, which would be a better choice

evolverine · 2020-04-02T10:33:21Z

agreed, a python library would be great for that.

…will do, even local ones

Stvad · 2020-04-12T05:43:15Z

crowd_anki/config/config_settings.py

        self.addon_manager = addon_manager or mw.addonManager
        self._config = init_values or addon_manager.getConfig(__name__)
+        self._window = window


this should be initialized similar to the addon_manage (i.e. be assigned mw by default) also probably should be a more specific dependency - does pm stands for something like profile_manager?

Do you mean like this? (see the latest commit)

crowd_anki/config/config_settings.py

crowd_anki/github/github_importer.py

katrinleinweber · 2020-04-12T20:37:50Z

[…] python lib […] for talking to Github API

[…] would be great for that.

Have you found developer.github.com/v3/libraries? It has a Python section, of which PyGithub seems to be quite API-complete :-)

…f string

…d at the provided URL

…ng profile_manager

evolverine · 2020-04-18T11:11:50Z

Thanks @katrinleinweber for researching the libraries. Can't wait to have this implemented. For now, let's finalize this pull request.

evolverine · 2020-04-29T17:38:32Z

@Stvad did you have a chance to take a look at the commits? Thanks.

Stvad · 2020-05-06T16:50:16Z

@evolverine I apologize for the delay, I thought I did, but I missed the latest batch. I'll take a look in the next few days!

crowd_anki/config/config_settings.py

Stvad · 2020-05-09T06:38:35Z

Looks good to me besides the small nit that I've noted!

…bject

…lt in the repository being instantiated without a parent folder

evolverine · 2020-05-12T09:07:26Z

great. And check out the last commit as well, where I made improvements in the way the repository URL is parsed. If that looks good, on my side it's all done (otherwise let me know if any changes are in order).

Stvad · 2020-05-13T03:05:02Z

crowd_anki/github/github_importer.py

@@ -8,6 +8,16 @@

 BRANCH_NAME = "master"

+
+def get_repository_name(repository_url):
+    repo_url_path = urlparse(repository_url).path


what does using urlparse give us?

true, not much. I thought it could help in cases when the URL contains query parameters, but nothing in github seems to.

Stvad · 2020-05-13T03:05:06Z

crowd_anki/github/github_importer.py

+    if repo_name.find(" ") != -1:
+        raise ValueError("repository name in URL should not contain spaces")


that's seems like an oddly specific thing to check for. should we just strip the string?

Took it out as well. I think the reason I was trying to validate the input so much was that Dulwich does create a folder and empty repository even when the URL is not correct, or does not contain a git repository. But we can handle that in other ways, perhaps.

…ameters. Also, checking for the space wasn't useful either; in that case, Dulwich will report an error Signed-off-by: Mihai Chira <mihai.chira@gmail.com>

evolverine · 2020-05-25T17:26:30Z

Heya, did you have a chance to look at the latest changes? Thanks!

Stvad · 2020-05-25T20:21:30Z

Hey, thank you for a reminder, been a bit swamped recently =. Will take a look today

Stvad · 2020-05-25T20:26:51Z

Looks good! Thank you for working on this! Sorry for a slow turn-around

evolverine · 2020-05-26T15:45:59Z

Yaay! Calls for a celebration 😃️🎉️

evolverine added 11 commits March 20, 2020 21:12

Stvad#46 adding GitPython source, including its dependencies

85fb7e2

Stvad#46 added the basic functionality to create and update a reposit…

b5a5f67

…ory by default instead of downloading the repository zip file

Stvad#46 marking the extra line of code that probably needs to be kep…

09d04f2

…t in place in future GitPython updates for the imports to work

Stvad#46 removing tests from GitPython sources and its dependencies

d416e9f

creating the repositories in folder structures that contain both the …

e72ef09

…username and repository name, to prevent name clashes

we actually have Dulwich already in use; no need for GitPython

d502c48

adding the .idea folder that PyCharm generates

7a8ccc2

Stvad#46 using Dulwich to clone and pull

94694a7

Stvad#46 going back to original directory structure until a discussio…

2ddca66

…n with author renders it safe to change

Stvad#46 going back to original directory structure until a discussio…

5467e86

…n with author renders it safe to change

merged with remote

16674d6

Stvad reviewed Mar 29, 2020

View reviewed changes

crowd_anki/utils/uuid.py Outdated Show resolved Hide resolved

Stvad reviewed Mar 29, 2020

View reviewed changes

evolverine added 4 commits April 2, 2020 12:35

Stvad#46 applying some of the changes suggested in the pull request

713b48a

Merge branch 'master' into 46_git_import

13f143d

Stvad#46 making the git importer server-agnostic; any git repository …

9a1bb88

…will do, even local ones

Stvad#46 cloning the git repository in the configured snapshot directory

a26537d

Stvad mentioned this pull request Apr 8, 2020

Save name of last used repository #22

Open

Stvad reviewed Apr 12, 2020

View reviewed changes

crowd_anki/config/config_settings.py Outdated Show resolved Hide resolved

crowd_anki/github/github_importer.py Outdated Show resolved Hide resolved

crowd_anki/github/github_importer.py Show resolved Hide resolved

crowd_anki/github/github_importer.py Outdated Show resolved Hide resolved

evolverine added 5 commits April 18, 2020 11:49

Stvad#46 renamed a variable for clarity, and made it a Path instead o…

45446d5

…f string

Stvad#46 extracted path manipulation into separate function

2a006e7

Stvad#46 extracted repository cloning into separate function

493c15b

Stvad#46 showing an error message when the git repository is not foun…

86486ff

…d at the provided URL

Stvad#46 making the dependency more specific - instead of window, usi…

187afaf

…ng profile_manager

Stvad reviewed May 9, 2020

View reviewed changes

crowd_anki/config/config_settings.py Outdated Show resolved Hide resolved

evolverine added 2 commits May 12, 2020 09:33

Stvad#46 passing in only the profile manager, instead of the window o…

71ac55f

…bject

Stvad#46 fixed a bug where a URL containing '/' at the end would resu…

c191b7c

…lt in the repository being instantiated without a parent folder

Stvad reviewed May 13, 2020

View reviewed changes

Stvad#46 urlparse wasn't helping, because github don't have query par…

e007183

…ameters. Also, checking for the space wasn't useful either; in that case, Dulwich will report an error Signed-off-by: Mihai Chira <mihai.chira@gmail.com>

Stvad merged commit a42041f into Stvad:master May 25, 2020

Stvad mentioned this pull request Jun 14, 2020

Error when trying to import decks #88

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#46 git import #78

#46 git import #78

evolverine commented Mar 25, 2020

Stvad Mar 29, 2020

evolverine Apr 6, 2020

evolverine Apr 10, 2020

Stvad Apr 12, 2020

evolverine Apr 18, 2020

Stvad commented Mar 30, 2020

evolverine commented Apr 2, 2020

Stvad Apr 12, 2020

evolverine Apr 18, 2020

katrinleinweber commented Apr 12, 2020

evolverine commented Apr 18, 2020

evolverine commented Apr 29, 2020

Stvad commented May 6, 2020

Stvad commented May 9, 2020

evolverine commented May 12, 2020

Stvad May 13, 2020

evolverine May 18, 2020

Stvad May 13, 2020

evolverine May 18, 2020

evolverine commented May 25, 2020

Stvad commented May 25, 2020

Stvad commented May 25, 2020

evolverine commented May 26, 2020

		if repo_name.find(" ") != -1:
		raise ValueError("repository name in URL should not contain spaces")

#46 git import #78

#46 git import #78

Conversation

evolverine commented Mar 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stvad commented Mar 30, 2020

evolverine commented Apr 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katrinleinweber commented Apr 12, 2020

evolverine commented Apr 18, 2020

evolverine commented Apr 29, 2020

Stvad commented May 6, 2020

Stvad commented May 9, 2020

evolverine commented May 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evolverine commented May 25, 2020

Stvad commented May 25, 2020

Stvad commented May 25, 2020

evolverine commented May 26, 2020