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

Import improvements #206

Closed
WithoutPants opened this issue Nov 15, 2019 · 8 comments · Fixed by #812
Closed

Import improvements #206

WithoutPants opened this issue Nov 15, 2019 · 8 comments · Fixed by #812
Assignees
Labels
help wanted Extra attention is needed improvement Something needed tweaking. investigate Investigation needed
Milestone

Comments

@WithoutPants
Copy link
Collaborator

There's a few issues with the import functionality as it is currently implemented.

  • import bombs out of importing objects as soon as it encounters an error, rolling back the entire transaction. In my opinion, objects should be imported in their own transaction, so the failure of importing one scene does not prevent the rest of the scenes from importing.
  • importing tags does not check for existing tags, so duplicate tags are created if they already exist
  • in my opinion, the importer should handle existing data, overwriting it with the imported data

Before I look at implementing these changes, I wanted to confirm if this is consistent with the general view of expected import behaviour.

@bnkai
Copy link
Collaborator

bnkai commented Nov 15, 2019

@WithoutPants afaik the importer was only meant for migrations and database schema upgrades.
The first thing it does is database.Reset() which removes the sqlite db file.
That's why it doesn't check for anything and probably why it packs everything in one transaction.
@StashAppDev is needed to verify though

Imho there should also be a separate non destructive import so that users could share perfomers,studios,tags for example.(i imagine scenes would be tricky with the path)

@StashAppDev
Copy link
Member

StashAppDev commented Nov 15, 2019

To echo what @bnkai said... the import should be dropping the database right at the start. It shouldn’t be creating duplicates and failing the entire import on error was intended. I wanted to ensure that no data was skipped from being imported so I was pedantic about errors. If we want to import without nuking current data I feel that should either be a new task or an option flag for the existing task.

Sent with GitHawk

@Leopere
Copy link
Collaborator

Leopere commented Nov 15, 2019

Ensure that there are sufficient UX notifications to let the end-user know that they are about to dump any preexisting data.

@Leopere Leopere added help wanted Extra attention is needed improvement Something needed tweaking. investigate Investigation needed labels Nov 15, 2019
@WithoutPants
Copy link
Collaborator Author

On my Windows machine, the database is definitely not deleted prior to import, so looks like there's a bug there.

I'll look at doing the update import as a separate mode.

@bnkai
Copy link
Collaborator

bnkai commented Nov 15, 2019

func Reset(databasePath string) { _ = DB.Close() _ = os.Remove(databasePath) Initialize(databasePath) }
On linux at least i verified that the sqlite db file gets deleted and created again.

@WithoutPants
Copy link
Collaborator Author

On Windows, I added logging and better error reporting if the database Reset fails and got the following output:

time="2019-11-16T19:58:28+11:00" level=error msg="Error resetting database: Error removing database: remove stash-go.sqlite: The process cannot access the file because it is being used by another process."

I tried adding a sleep between the Close and the os.Remove call, but even with a 5 second sleep, I get the same issue. So, on Windows at least it's not working as expected.

@ZZenvo
Copy link

ZZenvo commented Aug 3, 2020

Would like to suggest a feature/improvement related to this issue, hopefully adding more detail to it. Feature:

  • Ability to export and import 'parts' of the database. (Will call these parts 'DB-fragments' for now)

You'd then be able to share and obtain these DB-fragments. Example use-case:

  1. You want to upload a performer pack, and you've filled given scenes with sceneinfo.
  2. You select the scenes and export their entries/data to a DB-fragment, and include it in your upload.
  3. People finish downloading the scenes, then import the DB-fragment-file to populate downloaded scenes with data.

Would also be doable with single scenes ofc., and often you'd be the one to import.

As for the more technical side i can't speak too much, but i'd imagine that:

  • User finish downloading scene(s) ==> then scan ==> then import DB-fragment, where newly scanned hash would match with one in the DB-fragment, and then populate fields?
  • At the Tag and similair fields, a DB-fragment import could either overwrite or add. Possibly auto-create tags etc., if they aren't already existing. (similair to what ppl request for scraping functionality).

You guys already touched on some of these aspects, but hopefully i've added some more detail/goals.

@ZZenvo
Copy link

ZZenvo commented Aug 3, 2020

Would like to suggest a feature/improvement related to this issue, hopefully adding more detail to it. Feature:

  • Ability to export and import 'parts' of the database. (Will call these parts 'DB-fragments' for now)

You'd then be able to share and obtain these DB-fragments.

To further add a thought: I'd imagine that #454 would somewhat deprecate above-mentioned feature, but at the same time above-mentioned feature could possibly also save a lot of queries to the official stash-box instance, if that is a concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed improvement Something needed tweaking. investigate Investigation needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants