search

davidskalinder / mpeds-coder

MPEDS Annotation Interface
MIT License
0 stars 0 forks source link

Get hard-coded Python path out of generate-coder-table.py #125

Closed davidskalinder closed 3 years ago

davidskalinder commented 3 years ago

At the moment the first line of this is the path to the current Python bin, presumably to make the file executable and to make it so that cron doesn't have to worry about activating a particular conda env before running the thing.

Those are all good reasons to have it there. On the other hand, omg hard-coded paths omg srsly. (More specifically, it'll have to be changed every time the server environment changes, in many places if this solution proliferates.)

I dunno what the proper solution is here, but I do know that hard-coded paths in application code are hardly ever the proper solution. @alexhanna feel free to pipe up if you have any suggestions...

alexhanna commented 3 years ago

!!! Destroy it with fire. Actually I already have removed this in my fork I think.

On Fri, May 21, 2021, 08:36 davidskalinder @.***> wrote:

At the moment the first line of this is the path to the current Python bin, presumably to make the file executable and to make it so that cron doesn't have to worry about activating a particular conda env before running the thing.

Those are all good reasons to have it there. On the other hand, omg hard-coded paths omg srsly. (More specifically, it'll have to be changed every time the server environment changes, in many places if this solution proliferates.)

I dunno what the proper solution is here, but I do know that hard-coded paths in application code are hardly ever the proper solution. @alexhanna https://github.com/alexhanna feel free to pipe up if you have any suggestions...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davidskalinder/mpeds-coder/issues/125, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGC6DPXEQYUMBTZHQ4AVUTTOZ4ZDANCNFSM45JMGFBQ .

davidskalinder commented 3 years ago

Hah, gotcha. How does the cron job know which Python to run without it though?

alexhanna commented 3 years ago

Cronjob literally writes in the path of the correct Python. e.g. 5 0 * /opt/miniconda/bin/python /path/to/script.py

On Fri, May 21, 2021, 08:45 davidskalinder @.***> wrote:

Hah, gotcha. How does the cron job know which Python to run without it though?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davidskalinder/mpeds-coder/issues/125#issuecomment-846048103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGC6DJ4PA7K5ACNKY64SCTTOZ53FANCNFSM45JMGFBQ .

davidskalinder commented 3 years ago

Ah got it, yeah that's better.

If you've already taken it out in your branch, do you want to just PR it to yourself (or just change it upstream)? If not then I can do it and PR.

alexhanna commented 3 years ago

how about you do it? I didn't create a branch or anything for the change.

On Fri, May 21, 2021 at 9:07 AM davidskalinder @.***> wrote:

Ah got it, yeah that's better.

If you've already taken it out in your branch, do you want to just PR it to yourself (or just change it upstream)? If not then I can do it and PR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davidskalinder/mpeds-coder/issues/125#issuecomment-846068094, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGC6DPC57JDZV5UBNHXWE3TO2AM3ANCNFSM45JMGFBQ .

-- Alex Hanna, PhD alex-hanna.com Senior Research Scientist | Google Lecturer | UC Berkeley School of Information

davidskalinder commented 3 years ago

Okie doke, will do.

davidskalinder commented 3 years ago

Apparently this was fixed one way or another in 2bf92073955, so I'll close this.