Skip to content
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

Rewrite installer in Python 3 #3937

Closed
scolapasta opened this issue Jun 20, 2017 · 17 comments · Fixed by #6484
Closed

Rewrite installer in Python 3 #3937

scolapasta opened this issue Jun 20, 2017 · 17 comments · Fixed by #6484
Assignees

Comments

@scolapasta
Copy link
Contributor

@scolapasta scolapasta commented Jun 20, 2017

This is to support dev environments / installations in Windows.

@pdurbin

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin commented Jun 22, 2017

It pains me to say this, but I would suggest changing the title of this issue to "anything but Perl". Kids these days don't use it. 😢

I like Python but we should also consider Javascript, especially because since as of Java 8 there is a Javascript implementation called Nashorn we could use. Java is a hard requirement to run Dataverse. Python is only required if you want to build the docs with Sphinx or if you're hacking on geoconnect, dataverse-client-python, or miniverse. I can't think of a third language I'd suggest. Either Python or Javascript would do.

Please don't forget that it's really a Perl installer that calls into a series of Bash scripts. Let us consider if we are rewriting all those Bash scripts as well.

If the goal is truly to support Windows better, let's make sure we fully understand how well supported Python or Javascript or whatever is on Windows. I'd suggest that we check with @zhul-pku who opened #3927 and anyone who writes back on the Do you want to develop Dataverse on Windows thread I started on our mailing list.

Last thought: If we're re-writing the Dataverse installer not to use Perl, I assume we should also (in a future issue) re-write the TwoRavens installer not to use Perl: https://github.com/IQSS/TwoRavens/blob/master/install.pl

@zhul-pku

This comment has been minimized.

Copy link

@zhul-pku zhul-pku commented Jun 24, 2017

In the case of vagrant and virtualbox is used for Dataverse development in windows environment, it seems like there is no strong needs to rewrite the perl installer and bash scripts?
If the installer needs to be rewritten, and Python and Javacript both will do, I'd prefer Python. It is more popular nowadays, I took a course of Chinese language processing in Python recently, and more familiar with.

@bjonnh

This comment has been minimized.

Copy link

@bjonnh bjonnh commented Jun 26, 2017

I agree with @zhul-pku here. With vagrant (and the integration of ubuntu in the last windows), I'm not certain there is a need to rewrite anything.

There may be a need to clean-up and separate scripts by roles, and config. So we could eventually use the same scripts easily for dev and production. If it needs to be rewritten, I would go with python, because that's what I know best, but I don't think it matters much as long as it is working and doesn't add too many dependencies just to run an installer (which is why I will not vehemently defend porting the installer to anything else than bash and perl).

@pdurbin

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin commented Jul 13, 2018

@danmcp @thaorell and I talked about rewriting parts of the installer in Python for #4764 but in the end we decided to go with the approach of splitting the somewhat large Perl script into two. Smaller scripts are easier to rewrite so maybe we'll just keep doing this? Opinions are welcome.

@djbrooke djbrooke added this to Inbox 🗄 in IQSS/dataverse May 8, 2019
@djbrooke djbrooke added this to Ready 🙋 in IQSS/dataverse Oct 23, 2019
@djbrooke

This comment has been minimized.

Copy link
Contributor

@djbrooke djbrooke commented Oct 23, 2019

We discussed this in sprint planning 10/23. It will be good to revisit this as we are looking at the Glassfish upgrade. Instead of being focused on Python, we may want to make this effort more general and use case focused. I will let more technical folks make this decision. :)

@djbrooke djbrooke moved this from Ready 🙋 to IQSS Sprint 11/6 - 11/20 in IQSS/dataverse Nov 5, 2019
@djbrooke djbrooke changed the title Rewrite installer in Python Rewrite installer in Python 3 Nov 6, 2019
@djbrooke djbrooke added the Medium label Nov 6, 2019
@landreev landreev moved this from IQSS Sprint 11/6 - 12/4 to IQSS Team Dev 💻 in IQSS/dataverse Dec 2, 2019
@landreev landreev self-assigned this Dec 2, 2019
landreev added a commit that referenced this issue Dec 17, 2019
@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Dec 17, 2019

Massive kudos to @donsizemore for starting the effort 2 years ago. :)

@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Dec 17, 2019

remaining things to add/check on

  • Write release notes, update guide (if necessary)

  • Write a Python README (added requirements.txt - only Psycopg2?)

  • Add real-time validation for the admin email, mail server and glassfish directory values entered

  • Confirm all the modes supported by the old installer are still working (postgres-only; skip-database-setup; ...?)

  • Verify Python 3 compatibility

  • Rearrange code - move different setup functions into separate .py files (done; except maybe for some PostgresQL/psycopg2 code still in the main install.py?)

  • Confirm that it works in the context of spinning up AWS branches, in non-interactive mode

  • Add a check and a stern warning against running the script as root

  • Confirm that Docker-specific hooks work (this refers to the special installer mode added by our partners from BU, in the context of the "Cloud Dataverse" collaboration; various functions are triggered by special env. variables - MY_POD_NAME - so that the installer does different things on different nodes, in a multi-server installation. From what I'm seeing though this functionality may currently be broken in the old installer... so this may warrant a separate issue (?)

@pdurbin

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin commented Dec 18, 2019

@scolapasta and I spoke with @donsizemore today about how we'd like to exercise the new Python installer with our "spin up on AWS" scripts so I just created IQSS/dataverse-ansible#139 about this.

landreev added a commit that referenced this issue Dec 19, 2019
…atabase already exists. (#3937)
@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Dec 19, 2019

@donsizemore: @pdurbin mentioned you wanted some hooks/options added to the installer to be able to run on a system that has already been set up before.
I'm happy to add anything you need, just let us know how it should work.
There is already an option to continue even if the database already exists (--force/-f); and the option to skip the database setup completely (--skip_db_setup) - both of these were carried over from the old installer.
Should there be an option to wipe the db clean, then proceed with the install?
As I said, just let us know!

landreev added a commit that referenced this issue Dec 19, 2019
… warning from the old installer. (#3937)
landreev added a commit that referenced this issue Dec 19, 2019
landreev added a commit that referenced this issue Dec 19, 2019
landreev added a commit that referenced this issue Dec 19, 2019
…smtp server entered. (#3937)
@donsizemore

This comment has been minimized.

Copy link
Contributor

@donsizemore donsizemore commented Dec 24, 2019

@landreev this looks great, but I have another question: Python2 goes EOL on January 1st. Do we want to go ahead and make the jump to Python3? (sorry)

I volunteer to take that on if you'd like, now that you've done the heavy lifting.

@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Jan 3, 2020

@donsizemore I wanted to make it run under Python3, yes. (ideally, under both 2 and 3; but if it has to be Python3 only, should be ok too).
It's on my checklist.
I'll be back full-time on Monday, Jan 6, after an extended holiday break.

landreev added a commit that referenced this issue Jan 5, 2020
landreev added a commit that referenced this issue Jan 5, 2020
…e for making a distribution zip bundle. (#3937)
@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Jan 6, 2020

@donsizemore It should be working under Python3 now; while still working under Python2 as well. The reason I wanted backward compatibility with v2, even though it has been officially EOL-ed, was that there should still be many existing systems where Python2 is the default, or the only version, installed. We can discuss going forward whether this backward compatibility is something we want to maintain.
In the guides I mentioned that Python3 is recommended.
Please let me know if you notice anything I missed.

@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Jan 6, 2020

I have everything checked in and I'll go ahead and make a PR.
However I'm not sure about switching to this new installer as the default/recommended version immediately, in the upcoming release.
One of the reasons is that, despite the longer-term maintainability benefits, running the Python script actually requires more work on the part of the user: Perl is always part of the default distribution on any systems we support; Python, esp. v3, is not necessarily so. Python PostgreSQL client is not part of any standard distribution.
I'm not sure about forcing new users to perform these extra steps just for the sake of switching to something new. So my current plan is to advertise the new installer as an experimental alternative. The release notes, guides entries, etc. currently in the branch reflect that.
We can discuss this further if anyone has strong opinions.
(Another reason is that this new script is indeed new and experimental; and even after we run it through the QA, it may still end up being more buggy than the old workhorse. So it may help to advertise it as beta and collect input from the external users/developers before switching to it officially).

@djbrooke djbrooke removed this from IQSS Team Dev 💻 in IQSS/dataverse Jan 6, 2020
@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Jan 6, 2020

@pdurbin I'm going to open another ticket for the "podname"-based functionality. I replicated what was in the old installer (I think); but it really looks it's currently broken in the old installer - the code that's supposed to set up PostgreSQL appears to be inaccessible, due to faults in the logic.
So it needs to be revisited/retested.

I feel like it's (potentially) useful functionality and I do want to keep it, but I'm not sure if that was the best way to implement it - by relying on environmental variables inside the installer that is. I'm wondering if we'll be better off keeping it out of the installer, but adding a straightforward wrapper shell script, that would check and interpret the environmental variables, and pass them to the installer via the standard command line options and/or .config file entries.

Either way I feel it's logical to address this in a separate issue.
Will address your other suggestions in the PR.

landreev added a commit that referenced this issue Jan 8, 2020
landreev added a commit that referenced this issue Jan 8, 2020
landreev added a commit that referenced this issue Jan 8, 2020
landreev added a commit that referenced this issue Jan 9, 2020
@donsizemore

This comment has been minimized.

Copy link
Contributor

@donsizemore donsizemore commented Jan 10, 2020

@landreev running the script manually, I'm hitting:

Couldn't copy postgresql-42.2.2.jar into /usr/local/glassfish4/glassfish/modules. Check its permissions?

should the jar go in /usr/local/glassfish4/glassfish/lib?

@pdurbin

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin commented Jan 10, 2020

should the jar go in /usr/local/glassfish4/glassfish/lib?

I just checked with @sekmiller (who set up Payara on his laptop this week) and he said yes.

landreev added a commit that referenced this issue Jan 13, 2020
@landreev

This comment has been minimized.

Copy link
Contributor

@landreev landreev commented Jan 13, 2020

running the script manually, I'm hitting:

Couldn't copy postgresql-42.2.2.jar into /usr/local/glassfish4/glassfish/modules. Check its permissions?

should the jar go in /usr/local/glassfish4/glassfish/lib?

@donsizemore, that command line, with glassfish/modules was actually yours - it was part of what I copied from your Python installer draft, that you started last year. It looks like the driver can live in either glassfish/modules or glassfish/lib. I never ran into any problems with the former location in my testing because I usually just chown the entire glassfish directory to whatever user will be running it, and not just the select directories.
I'll go ahead and change it back to glassfish/lib - because we specifically mention it in the guide; and somebody else would likely run into this problem too.
Thanks.

landreev added a commit that referenced this issue Jan 13, 2020
…ded; if you really really have to use v2. - 2.7+ required) (#3937)
landreev added a commit that referenced this issue Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.