Islandora-Devops / isle-dc

ISLE 8 - Dockerized Islandora 8 Deployment orchestrated with docker-compose
MIT License
23 stars 60 forks source link

let demo_content target in makefile run multiple times #295

Closed phette23 closed 2 years ago

phette23 commented 2 years ago

To test:


I ran into some trouble building the demo site and had to run make demo multiple times, but on iterations after the first I kept seeing this error related to running setup.py in Islandora workbench:

File "/workbench/setup.py", line 7
    author_email="mjordan@sfu", packages=["i7Import", "i8demo_BD", "input_data"], packages=["i7Import", "i8demo_BD", "input_data"],
                                                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: keyword argument repeated: packages

I tracked the problem back to the final sed command in the demo_content target; it replaces "author_email="mjordan@sfu", with "author_email="mjordan@sfu", packages=[... but it's happy to do that multiple times, leading to the error above. Most other things in the repeat run seemed to work despite some warnings. I just modified the sed so it'll only run the substitution on the line if the line doesn't already have "packages" listed. I also removed sed's "g" flag since I don't think a global substitution makes sense here.

I realize this is an edge case since people shouldn't be running make demo multiple times without cleaning things out in between but it came up for me so I figured I'd submit a PR.

alxp commented 2 years ago

I was able to run "make demo_content" twice in a row with no occurrence of the "syntax error" message.

Un-assigning myself since I'm not an ISLE expert but it seems OK to merge regardless if it is reproducible elsewhere.

adam-vessey commented 2 years ago

PR is presently in conflict; however, could this potentially be due to the other issue, with different flavours of sed?... though #291 also indicates that it addresses the idempotency of the sed invocation, so this may no longer be an issue?

phette23 commented 2 years ago

I just tested with a fresh isle-dc at HEAD on my laptop and running make demo_content twice in a row didn't throw any errors. The other sed edit in f44ddc1838bfd64fcc3d34b0976527a9afe75e9c fixed this because it added $$ to the end of the sed substitution; now the second run won't match the substitution pattern because the comma after the email is no longer the end of the line.

So I'll close this PR, there's no need to merge it. The $$ change is less text, more elegant anyways.