-
Notifications
You must be signed in to change notification settings - Fork 522
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
Add text column db #209
base: master
Are you sure you want to change the base?
Add text column db #209
Conversation
Awesome. I will look at it. Thanks for your contribution |
Have you tested on iOS yet? On the iOS side, to modify the database schema, you need to update the sql file too. And what about DB migration? If current projects upgrade to this new DB, does it still work? |
Btw, I've used sqlitebrowser to create that sql file |
Hi hnvn. I think this is necessary in most projects which uses flutter downloader. |
Working with SQLite in iOS is painful. I concern that modifying DB schema might break the current features. |
Ok, Thanks a lot hnvn for attention. |
Thanks for your contribution. My priority now is to migrate this plugin to embedding v2, then I will get back to this issue and try to find a solution for it. |
Thanks for your plugin migration. |
@mohammadne Do you use this to store json data? I'm looking for something that could do that instead of writing it externally and keeping two separate date sources in sync. |
any Update on this ??? |
If you need to store additional information related to a task (and that information is not related to downloading the task) then I think you should create an additional database or map that has taskId as its key and whatever fields you want to add. I don't think you should add functionality or features to the downloader task database that are not strictly required for or related to the download functionality |
Sorry to bump? |
Hi, I think the better idea is to define a list of parameters like Map<String,String> and before storing in database use method like "json_encode" to convert that list to string, and when retrieving from database convert it back to list by "json_decode". I'm looking forward to hearing from you guys. |
Hey @hnvn, what must be done so this PR gets merged? I also need it and it seems that most of the job is done here, isn't it? I am willing to contribute if I can. |
What are you afraid of @hnvn? Since this is a new column I don't see how it could break any feature as no pre-existing code rely on it. I see that at the moment the plugin simply deletes the previous database in Android when a migration of the database is required ( |
Hi, is this a tested fork, does it work on both android and ios, if it does it should be merged with the main project, it a very crucial features, how are we supposes to track what our apps download if there is noting unique we can add in the tasks db, so pls @hnvn can you inform us if we should use this fork or if we should wait till it can be merged. |
Alas it seems that nothing has happen on this matter. I have had to implement my own persistent db in parallel to keep track of the "loading task"/"app entity" mapping and it has been a real pain to keep it in sync with the plugin's one. If someone could finish this PR all this work could be avoided (I didn't because I am not myself an iOS developer). I had created another issue where I describe the problems that such a feature would solve: #402. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR. Please take a look at my comments and fix the merge conflicts.
Also, please consider implementing improvements mentioned in this comment.
<?xml version="1.0" encoding="UTF-8"?> | ||
<projectDescription> | ||
<name>android</name> | ||
<comment>Project android created by Buildship.</comment> | ||
<projects> | ||
</projects> | ||
<buildSpec> | ||
<buildCommand> | ||
<name>org.eclipse.buildship.core.gradleprojectbuilder</name> | ||
<arguments> | ||
</arguments> | ||
</buildCommand> | ||
</buildSpec> | ||
<natures> | ||
<nature>org.eclipse.buildship.core.gradleprojectnature</nature> | ||
</natures> | ||
</projectDescription> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like an autogenerated file included by mistake - please delete it.
arguments= | ||
auto.sync=false | ||
build.scans.enabled=false | ||
connection.gradle.distribution=GRADLE_DISTRIBUTION(VERSION(6.0-20191016123526+0000)) | ||
connection.project.dir= | ||
eclipse.preferences.version=1 | ||
gradle.user.home= | ||
java.home=/usr/lib/jvm/java-11-openjdk-amd64 | ||
jvm.arguments= | ||
offline.mode=false | ||
override.workspace.settings=true | ||
show.console.view=true | ||
show.executions.view=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
String headers, String mimeType, boolean resumable, boolean showNotification, boolean openFileFromNotification, long timeCreated) { | ||
this.primaryId = primaryId; | ||
this.taskId = taskId; | ||
this.status = status; | ||
this.progress = progress; | ||
this.url = url; | ||
this.filename = filename; | ||
this.additionalinfo = additionalinfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rename this field to extra
.
{ | ||
// Use IntelliSense to learn about possible attributes. | ||
// Hover to view descriptions of existing attributes. | ||
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 | ||
"version": "0.2.0", | ||
"configurations": [ | ||
{ | ||
"name": "Flutter", | ||
"request": "launch", | ||
"type": "dart" | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete this file.
<?xml version="1.0" encoding="UTF-8"?> | ||
<projectDescription> | ||
<name>android</name> | ||
<comment>Project android created by Buildship.</comment> | ||
<projects> | ||
</projects> | ||
<buildSpec> | ||
<buildCommand> | ||
<name>org.eclipse.buildship.core.gradleprojectbuilder</name> | ||
<arguments> | ||
</arguments> | ||
</buildCommand> | ||
</buildSpec> | ||
<natures> | ||
<nature>org.eclipse.buildship.core.gradleprojectnature</nature> | ||
</natures> | ||
</projectDescription> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete.
<?xml version="1.0" encoding="UTF-8"?> | ||
<classpath> | ||
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11/"/> | ||
<classpathentry kind="con" path="org.eclipse.buildship.core.gradleclasspathcontainer"/> | ||
<classpathentry kind="output" path="bin/default"/> | ||
</classpath> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
<?xml version="1.0" encoding="UTF-8"?> | ||
<projectDescription> | ||
<name>app</name> | ||
<comment>Project app created by Buildship.</comment> | ||
<projects> | ||
</projects> | ||
<buildSpec> | ||
<buildCommand> | ||
<name>org.eclipse.jdt.core.javabuilder</name> | ||
<arguments> | ||
</arguments> | ||
</buildCommand> | ||
<buildCommand> | ||
<name>org.eclipse.buildship.core.gradleprojectbuilder</name> | ||
<arguments> | ||
</arguments> | ||
</buildCommand> | ||
</buildSpec> | ||
<natures> | ||
<nature>org.eclipse.jdt.core.javanature</nature> | ||
<nature>org.eclipse.buildship.core.gradleprojectnature</nature> | ||
</natures> | ||
</projectDescription> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete.
connection.project.dir=.. | ||
eclipse.preferences.version=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😕 please remove this file
@@ -1,5 +1,6 @@ | |||
#!/bin/sh | |||
# This is a generated file; do not edit or check into version control. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rever this change.
@@ -34,6 +34,7 @@ class DownloadTaskStatus { | |||
/// * [progress] the latest progress value of a download task | |||
/// * [url] the download link | |||
/// * [filename] the local file name of a downloaded file | |||
/// * [additionalInfo] Additional information of a downloaded file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change to something like Optional, extra string associated with this task
in my use case I have to add additional info to task table (very necessary for me).
refer to issue #195
so I open my first pull request in open source.
then I try to download but it will fail.
if anyone can help me that's a good encouragement for me.
Thanks to the community.