support conversion progress #68
Conversation
rawdaGastan
force-pushed
the
development_support_conversion_progress
branch
from
ce69c5e
to
e1ce143
Compare
rawdaGastan
force-pushed
the
development_fl_server
branch
from
6f217ec
to
df36948
Compare
…into development_support_conversion_progress
rawdaGastan
force-pushed
the
development_support_conversion_progress
branch
from
e450cfc
to
f867407
Compare
…into development_support_conversion_progress
rawdaGastan
force-pushed
the
development_fl_server
branch
from
3d4f18a
to
e588b80
Compare
There was a problem hiding this comment.
Thank you Rawda for create the PR. but i have few comments you can see on the PR. I would always recommend in general to document your code (special docs over function names to make it easier to follow the logic.
I was also wondering if the rfs library should provide a better progress callback mechanism than just a channel that you send number of files over. And every user of the rfs library can then render progress directly. For example a CLI can show directories, file names, progress of file download etc. While a WEB ui like yours can show just number of files processes. (maybe later?)
A more important note is that in many cases where u use copy a shared reference would be more than enough.
…into development_support_conversion_progress
…into development_support_conversion_progress
rawdaGastan
force-pushed
the
development_fl_server
branch
2 times, most recently
from
056aabf
to
2efda5e
Compare
…into development_support_conversion_progress
docker2fl/src/docker2fl.rs
Outdated
| self.files_count = WalkDir::new(self.docker_tmp_dir_path.as_path()) | ||
| .into_iter() | ||
| .count(); |
There was a problem hiding this comment.
Since you doing extract already is it possible to make extract_image return the number of files instead of doing this iteration (that can be intensive) just to return the number of files.
| @@ -268,12 +334,13 @@ pub async fn get_flist_state_handler( | |||
| pub async fn list_flists_handler(State(state): State<Arc<config::AppState>>) -> impl IntoResponse { | |||
There was a problem hiding this comment.
does this list all flists or per user? what if we have too many of them? should you consider pagination ? and or filtering as well.
Does it make sense to use some small db (say sqlite) to track the flists instead of scanning the entire directory of all flists everytime ?
The db can be recreated from fs when/if needed but it will be way more efficient to hit the db for queries like what flists we have, what users, what flists for that user, etc.. than scanning the fs
docker2fl/src/docker2fl.rs
Outdated
| command = (entrypoint.first().unwrap()).to_string(); | ||
| let entrypoint = container_config | ||
| .entrypoint | ||
| .expect("failed to get entry point"); |
There was a problem hiding this comment.
Why this should cause the entire process to panic? What if the container I am using is not configured with an entrypoint.
I really think all expect and unwrap here should instead return a suitable errors that can be handled instead of panicing
rawdaGastan
force-pushed
the
development_support_conversion_progress
branch
from
9d61ac7
to
70b4933
Compare
…t_support_conversion_progress
rawdaGastan
force-pushed
the
development_support_conversion_progress
branch
from
70b4933
to
674ca75
Compare
Issues