- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 7
Improvements to onboard_project #848
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
Conversation
-Can now onboard multiple projects - --copy-from now will use the user's Downloads folder if included with no value -clean_project changed from silnlp.scripts.clean_project to silnlp.common.clean_projects so multiple projects can be cleaned at once and more unncessary files are removed -Added a wildebeest section to the config, and default args for wildebeest analysis
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.
@benjaminking reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mmartin9684-sil)
silnlp/common/onboard_project.py line 170 at r1 (raw file):
"--wildebeest", default=False, action="store_true", help="Run Wildebeest analysis on the extracted corpora." ) parser.add_argument(
How does this work when the user passes multiple projects? Can they pass multiple zip files with different passwords?
silnlp/common/onboard_project.py line 200 at r1 (raw file):
if needs_password: if args.zip_password: pwd = args.zip_password
I'm guessing that this change was driven by a request from the EITL team, but I liked prompting the user for the passwords, to help keep passwords out of the command history. But if EITL requested it to work this way, we should go with that.
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.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @benjaminking and @mmartin9684-sil)
silnlp/common/onboard_project.py line 170 at r1 (raw file):
Previously, benjaminking (Ben King) wrote…
How does this work when the user passes multiple projects? Can they pass multiple zip files with different passwords?
No, it would not work. I have changed this to instead use a zip_password section in the config. The config would look something like this:
Code snippet:
zip_password:
    project_name_1: password_1
    project_name_2: password_2silnlp/common/onboard_project.py line 200 at r1 (raw file):
Previously, benjaminking (Ben King) wrote…
I'm guessing that this change was driven by a request from the EITL team, but I liked prompting the user for the passwords, to help keep passwords out of the command history. But if EITL requested it to work this way, we should go with that.
Yes, because the script may take some time to run, it would be inconvenient to have to watch for any password prompts.
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.
@benjaminking reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @mmartin9684-sil)
silnlp/common/onboard_project.py line 170 at r1 (raw file):
Previously, TaperChipmunk32 (Matthew Beech) wrote…
No, it would not work. I have changed this to instead use a
zip_passwordsection in the config. The config would look something like this:
I like this solution.

--copy-fromnow will use the user's Downloads folder if included with no valuesilnlp.scripts.clean_projecttosilnlp.common.clean_projectsso multiple projects can be cleaned at once and more unnecessary files are removedThis change is