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

WIP *DO NOT MERGE*: new-script/httpcrtsrv #15

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

jbh
Copy link
Collaborator

@jbh jbh commented Jan 10, 2018

This is a WIP. Do not merge until it is complete.

Right now, this SQL has no effect when ran via Python:

create or replace alias QUSRSYS.QATMHINSTC_test for QUSRSYS.QATMHINSTC(test);
update QUSRSYS.QATMHINSTC_test
set CHARFIELD = '-apache -d /www/test -f conf/httpd.conf -AutoStartN';

When run outside of Python, it works and the correct httpd.conf is pointed to.

I would prefer to use the QzuiCreateInstance API, but I would need help using the Python Toolkit. Any suggestions @kadler?

@kadler
Copy link
Contributor

kadler commented Jan 10, 2018

Haven't looked at the code yet, but you say the SQL doesn't work in Python. Seems odd that it wouldn't. Perhaps you need to turn off commitment control or commit it or turn on autocommit (ibm_db_dbi has autocommit off by default to comply with PEP 249).

@jbh
Copy link
Collaborator Author

jbh commented Jan 10, 2018

I do a commit. I believe the SQL runs, it's just that no rows are affected by it. I thought maybe the record wasn't created yet, but I ran tests to make sure that wasn't the case. I'd rather not do it the SQL way, but it should have an effect regardless.

@@ -0,0 +1,444 @@
#!/usr/bin/env python3
import optparse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optparse is deprecated, better to use argparse instead

p.add_option('--port', '-p', help="Default port for HTTP Server Instance")
options, arguments = p.parse_args()

if len([x for x in (options.create, options.delete, options.rename) if x is not None]) != 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to do this with a mutually exclusive argument group: https://docs.python.org/3/library/argparse.html#mutual-exclusion


if options.create:
if len([x for x in (options.name, options.conf, options.port) if x is not None]) == 3:
if options.conf in ['apache', 'zendphp7', 'zendsvr6']:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be accomplished with the choices parameter to add_argument

p.error('options -c, -d, and -r cannot be used together.')

if options.create:
if len([x for x in (options.name, options.conf, options.port) if x is not None]) == 3:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be done with subparsers, but you would have to change to using positional arguments instead of flags for --create, --rename, and --delete:

python3 httpsrv.py -c --conf=zendsvr6 --name=test --port=10090
python3 httpsrv.py -r --name=test --newname=develop
python3 httpsrv.py -d --name=develop

to

python3 httpsrv.py create --conf=zendsvr6 --name=test --port=10090
python3 httpsrv.py rename --name=test --newname=develop
python3 httpsrv.py delete --name=develop


verified = input(verification_text) or 'Y'

if verified in ['y', 'Y', 'yes', 'Yes', 'YES']:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if verified.lower() in ('y', 'yes')

fastcgi_conf_file.close()
fastcgi_conf_twn_file.close()
fastcgi_dynamic_conf_file.close()
fastcgi_http_add_conf_file.close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend for the above to open the files using the with statement (as a context manager) so you don't have to manually close them:

with open("%s/conf/fastcgi.conf" % path, "w+") as fastcgi_conf_file:
    fastcgi_conf_file.write(get_fastcgi_conf(name))

LogMaintHour 3

IncludeOptional /www/%s/conf/apache-sites/*.conf
''' % (port, name, name, name, name, name, name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want a healthy dose of pyformat throughout the file, but the %% get really bad here. Really, though, Python 3.6 and f-strings can't come soon enough!

elif conf == "zendphp7":
conf_file.write(get_zendphp7_conf(name, port))
elif conf == "zendsvr6":
conf_file.write(get_zendsvr6_conf(name, port))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do this dynamically:

get_conf = globals()['get_{}_conf'.format(conf)]
with open("%s/conf/httpd.conf" % path, "w+") as conf_file:
    conf_file.write(get_conf(name, port))

create or replace alias QUSRSYS.QATMHINSTC_%s for QUSRSYS.QATMHINSTC(%s);
update QUSRSYS.QATMHINSTC_%s
set CHARFIELD = '-apache -d /www/%s -f conf/httpd.conf -AutoStartN';
''' % (name, name, name, name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't execute multiple queries at once in ibm_db_dbi. You will have to split these in to two queries:

mod_info_query = 'create or replace alias QUSRSYS.QATMHINSTC_{0} for QUSRSYS.QATMHINSTC({0})'.format(name)
cur.execute(mod_info_query)

mod_info_query = "update QUSRSYS.QATMHINSTC_{0} set CHARFIELD = '-apache -d /www/{0} -f conf/httpd.conf -AutoStartN'".format(name)
cur.execute(mod_info_query)

@jbh
Copy link
Collaborator Author

jbh commented Jan 10, 2018

There are still a few I need to take care of, as well as making sure to use one sql statement per statement execution (duh). I'll get to that by EOD today. Thanks for the tips and good code review @kadler.

@jbh
Copy link
Collaborator Author

jbh commented Jan 11, 2018

All done with Kevin's suggestions. The SQL works fine now. Subparsers make the code much cleaner.

Testing needs to be done. The HTTP Server Instance starts, but it doesn't work properly. I have a feeling it has to do with the configuration files being generated. Besides that, the rename and delete commands need to be completed.

Copy link
Contributor

@kadler kadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. Will be exciting to see it actually work.

LogFormat "%{{Cookie}}n \\"%r\\" %t" cookie
LogFormat "%{{User-agent}}i" agent
LogFormat "%{{Referer}}i -> %U" referer
LogFormat "%h %l %u %t \\"%r\\" %>s %b" common
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got rid of the having to escape the %, but now we have to escape the {}... LOL

os.mkdir('{}/conf'.format(path))
os.mkdir('{}/logs'.format(path))
os.mkdir('{}/htdocs'.format(path))
index_file = open("{}/htdocs/index.html".format(path), "w+")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was gonna comment before that for these that are simple concatenation, it probably makes more sense to just concatenate them:

path+'/htdocs/index.html

vs

"{}/htdocs/index.html".format(path)

UseCanonicalName On
TimeOut 30000
KeepAlive On
KeepAliveTimeout
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a value on KeepAliveTimeout:

Cause . . . . . :   Directive KeepAliveTimeout does not have the correct      
   number of parameters.  The HTTP server did not start. Recovery  . . . :   
   Correct the directive and start the HTTP server again. Technical description
   . . . . . . . . :   See the HTTP server documentation on configuration and  
   administration for more information.   
    01/10/18  22:51:04.568293  QZSRAPR      QHTTPSVR    *STMT    QZSRCORE    QH

MaxDynamicServers 100


;Usage of this configuration requires follwoing PASE and DG1 PTFs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo on follwoing

Options FollowSymLinks
order allow,deny
allow from all
AllowOverride all
Copy link
Contributor

@kadler kadler Jan 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order allow,deny and order deny,allow don't seem to work on IBM i 7.2 for me. I had to change them to Require all granted and Require all denied instead.

If you want to support 7.1 and 7.2 syntax you can query uname:

import os
un = os.uname()
version = (int(un.version), int(un.release))
if version > (7,1):
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of the latest commit? Mine keeps timing out still.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once I added the KeepAliveTimeout, the server started and then once I changed the Apache 2.2 permission syntax to Apache 2.4 syntax it worked for me.

@kadler
Copy link
Contributor

kadler commented Jan 11, 2018

Basis for calling QzuiCreateInstance: https://gist.github.com/kadler/cb31c1b35129d4b0747065f3e6cb303a

# Could not find a command for this one. Only idea is to use CHGPF or some command to change the member name
# rerun command to update update apache configuration path
# Then mv directory to rename directory


def start(name):
os.system("system 'STRTCPSVR SERVER(*HTTP) HTTPSVR(" + name.upper() + ")'")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I could just write:

os.system('STRTCPSVR SERVER(*HTTP) HTTPSVR(' + name.upper() + ')')

The above wouldn't work, though. It seems to be running system commands under bash instead of IBM i level.

@jbh
Copy link
Collaborator Author

jbh commented Jan 11, 2018

It does produce the error you describe in the notes.

.addData(iData('out_table_name', '10a', '*GLOBAL'))
.addData(iData('out_table_lib', '10a', '*GLOBAL'))
.addData(iData('in_table_name', '10a', '*GLOBAL'))
.addData(iData('in_table_lib', '10a', '*GLOBAL'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the docs, these should be blank when the name is *GLOBAL. I messed that up as I was writing my test.

@kadler
Copy link
Contributor

kadler commented Jan 11, 2018

FYI, I updated the gist with new notes as I was playing around.

.addParm(iData('instance', '10a', name.upper()))
.addParm(
iDS('INSD0110', {'len': 'buflen'})
.addData(iData('autostart', '10a', '*NO'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the QHTTPSVR/H(QZHBCONF) header file, it seems the structure is not packed, so we need to add 2 bytes of padding here so that the integers following are aligned on a 4-byte boundary (offset 12 instead of 10).

                    .addData(iData('pad_for_alignment', '2h', '0'))

itool.add(iCmd('addlible', 'addlible QHTTPSVR'))
itool.add(
iSrvPgm('QzuiCreateInstance', 'QZHBCONF', 'QzuiCreateInstance',
iopt={'lib': 'QHTTPSVR'}) # This doesn't seem to work, thus the ADDLIBLE above
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks again Kevin. Sorry I fell asleep on ya!

@jbh
Copy link
Collaborator Author

jbh commented Jan 12, 2018

All the commands work now, but they use SQL combined with commands instead of the APIs.

Commands working:

httpsrv create
httpsrv rename
httpsrv delete
httpsrv start
httpsrv restart
httpsrv stop

@kadler
Copy link
Contributor

kadler commented Jan 23, 2018

What still needs to be done for this PR? Just the TODOs listed in f9688ae?

@jbh
Copy link
Collaborator Author

jbh commented Jan 23, 2018

@kadler that's correct. Just the TODOs. Mainly the fact that the instance created doesn't just run. I haven't decided if it's IFS permissions or what, but the connection times out when going to the instance in a browser. It needs more testing.

@kadler
Copy link
Contributor

kadler commented Jan 24, 2018

Odd, because it worked for me when I tested it. If I have some time, I'll try it out again.

@jbh
Copy link
Collaborator Author

jbh commented Jan 24, 2018

Were you testing the PHP 7 config? That's what I was testing. I'll give it another go today.

@kadler
Copy link
Contributor

kadler commented Jan 24, 2018

Oh no, just the HTTP server config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants