flickr-uploader icon indicating copy to clipboard operation
flickr-uploader copied to clipboard

Added REMOVE_PATH_PARTS option

Open bezineb5 opened this issue 6 years ago • 7 comments

Hi, I added a feature to manipulate the name of the path before computing the album name. Example: REMOVE_PATH_PARTS = ["/Output$","/\d{4}-(0[1-9]|1[0-2])-([0-9]{2})$"] The first regex will remove the "Output" name from the path, taking the parent name it's useful when working with Capture One sessions): "/folder/myAlbumName/Output" -> "myAlbumName" The second regex will remove the date from the path: "/folder/myAlbumName/2018/10/12" -> "myAlbumName" Obviously, it's configurable as it's regex-based.

I also added the name in the album in the dry run output and added a hidden folder created by QNAP NAS devices.

What do you think about it? Do you see that as a useful addition? Thanks, Ben

bezineb5 avatar Oct 19 '18 14:10 bezineb5

First-off thanks for the changes/suggestion. 😄

Concept

  • Sounds interesting but also a bit risky, no? A change in "REMOVE_PATH_PARTS" in-between runs may mean a delete/reupload of an entire library.
    • So the README has to be explained very well.

Tests...

  • I'm running the basic tests I have. For now some pics are being duplicated in some test cases... Have to check!
  • May have to add some specific tests for this scenario.
  • Will let you know how things go...

Relation with FULL_SET_NAME. Help in testing...

  • Could you help out with one test... which is with the relation with the setting FULL_SET_NAME.
    • We may run in into a situation with FULL_SET_NAME=False and REMOVE_PATH_PARTS=["/Sub$"] and a pic /home/user/media/Album4/Sub/pic041.jpg that the result SetName is in fact NULL.
    • That would be an issue.
    • In the example you show I think you're assuming FULL_SET_NAME=False... so in the second example Album4/Sub we have to test if it actually returns Album4 or NULL.
FilePathName Set/Album Name (without REMOVE_PATH_PARTS) Set/Album Name (with REMOVE_PATH_PARTS)
/home/user/media/Album4/pic04.jpg Album4 Album4
/home/user/media/Album4/Sub/pic041.jpg Sub Album4 ❗️ Confirm

Is this a typo?

  • ❓Quick question: On README this is a typo correct: name to anem the album

oPromessa avatar Oct 19 '18 23:10 oPromessa

@bezineb5 I was going thru testing your code but I am getting the following error:

  • sqlite3.InterfaceError: Error binding parameter 2 - probably unsupported type.

When executing the following code. Only reason I see is the self.xcfg.REMOVE_PATH_PARTS being a list and not supported by sqlite3. Did I screw up your code in any way or which python version are you using so that it works?

            litedb.execute(con, 'SELECT#145', slockdb, self.args.processes,
                           cur,
                           'SELECT DISTINCT getSet(path, ?, ?, ?) '
                           'FROM files WHERE getSet(path, ?, ?, ?) '
                           'NOT IN (SELECT name FROM sets)',
                           qmarkargs=(self.xcfg.FILES_DIR,
                                      self.xcfg.FULL_SET_NAME,
                                      self.xcfg.REMOVE_PATH_PARTS,
                                      self.xcfg.FILES_DIR,
                                      self.xcfg.FULL_SET_NAME,
                                      self.xcfg.REMOVE_PATH_PARTS,),
                           dbcaughtcode='145')

oPromessa avatar Oct 20 '18 21:10 oPromessa

Hi @oPromessa , Thanks for having a look at it! To answer your question, the change removes the part before finding the album name (the regular expression is not applied on the album name but in the path). So in the example /home/user/media/Album4/Sub/pic041.jpg, the path will be /home/user/media/Album4/Sub, so after the regexp deletion, only /home/user/media/Album4 will stay. Hence the album name will be Album4. Does that make sense? My goal is to deal with Capture One sessions, where output jpegs are in the Output folder of what I want to be an album. Also, on some albums, I have a split by date, and I want to discard that structure on Flickr albums.

Concerning the DB storage, I think that indeed, I didn't try that part - it seems to be run only after a real flickr import, and not a dry run, right? My bad, sorry for that. I've commited a fix, but I've seen you have tests, how can I run them locally?

bezineb5 avatar Oct 21 '18 05:10 bezineb5

@bezineb5 Had a quick look at your fix and it's good. Will check.

My tests have on Travis-ci.org environment have the api_key/secret values for a flickr account I have. Such settings are defined as "private" settings within Travis-CI environment. That's why they simply don't run with your user.

  • I guess for the tests to run for you, you would have to associate your repository with www.travis-ci.org and define those exact same settings (I can send the exact list to you if you want). Then all the instructions in .travis.yml should work. -- Never tried it but it should work.
    • I upload the resulting logs to a DropBox account... so if you want that... you also need to setup some access to such Dropbox account. Of course those are private settings within your own Travis-Ci environment.
  • Alternatively what I do is I have a ubuntu 16 virtual machine where I:
    • git clone the repository
    • run python setup.py develop
    • and then I can run it from a "Test folder" by setting upload.ini values:
      • api/secret, etc...
      • The FILES_DIR you can point out to the <repository folder>/tests/Test Photo Library which holds my test pics.

oPromessa avatar Oct 21 '18 10:10 oPromessa

Coding...

I've put some protection on the set_name_from_file function.

  • Not finished yet but you could check the code and the test cases (on docstrings) and the introduction of a scenario for NoAlbum situations.

This can be an issue

Coming back to the example I mention: the problem is when the regular expression is such that you end up with a NULL value for set_name. Those are the protections I'm trying to make. of course one should configure REMOVE_PATH_PARTS correctly but it is prone to user error.

["/Sub$"]and a pic/home/user/media/Album4/Sub/pic041.jpg` that the result SetName is in fact NULL.

  • That would be an issue.
  • In the example you show I think you're assuming FULL_SET_NAME=False... so in the second example Album4/Sub we have to test if it actually returns Album4 or NULL.
FilePathName FILES_DIR FULL_SET_NAME = False REMOVE_PATH_PARTS Set/Album Name (with REMOVE_PATH_PARTS) Set/Album Name (Code Protected) Remark
/home/user/media/Album4/pic04.jpg /home/user/media False Album4 media As it would be NULL. I'm assuming the basedir folder from FILES_DIR ("media")
/home/user/media/Album4/Sub/pic041.jpg /home/user/media True Sub Album4 Album4
/home/user/media/Album4/Sub/pic041.jpg /home/user/media True 'Sub', 'Album4' media Again the need to protect the empty value. I'm assuming the basedir folder from FILES_DIR ("media")

oPromessa avatar Oct 27 '18 13:10 oPromessa

Good catch for the exception handling!

In the docstring, I'm not sure about the regular expression: '/some/photos', False, ['(/Out)+']) I'd keep the '$' to indicate the end of line, instead of the '+'

I 'm also thinking that the base path should be protected, to avoid issue when computing the relative path. I'll have a look at this.

In the case the path ends up being empty, what do suggest to do? I would say that in this case we should have the same behaviour as when a picture is located at the root of the FILES_DIR. How would an album be named for a picture called /home/user/media/pic041.jpg?

By the way, I tested on my data set, and the album creation works fine in the general cases.

bezineb5 avatar Oct 27 '18 13:10 bezineb5

In the case the path ends up being empty, what do suggest to do? I would say that in this case we should have the same behaviour as when a picture is located at the root of the FILES_DIR. How would an album be named for a picture called /home/user/media/pic041.jpg?

My thoughts exactly... in such case the album is set to "media". That was already the case prior to the REMOVE_PATH_PARTS setting. So I'm inclined to keep it.

Thanks for the feedback on the tests!

oPromessa avatar Oct 27 '18 14:10 oPromessa