-
Notifications
You must be signed in to change notification settings - Fork 28
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: Add some more flexibility #8
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,24 @@ | |
from __future__ import absolute_import | ||
from __future__ import print_function | ||
|
||
from .server import local_file_location | ||
|
||
|
||
class PPD(object): | ||
def text(self): | ||
raise NotImplementedError() | ||
|
||
|
||
class FilePPD(PPD): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably make all the other PPD classes into normal files too - setup.py handles data_files, so this shouldn't be too bad. But I won't force you to do that as part of this PR! |
||
def __init__(self, filename): | ||
filepath = local_file_location(filename) | ||
with open(filepath, "r") as ppdfile: | ||
self.ppd = ppdfile.read() | ||
|
||
def text(self): | ||
return self.ppd.encode("ascii") | ||
|
||
|
||
class BasicPostscriptPPD(PPD): | ||
product = 'ipp-server' | ||
manufacturer = 'h2g2bob' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,9 @@ | |
|
||
|
||
def local_file_location(filename): | ||
filename = os.path.basename(filename) | ||
if os.environ.get("IPP_DATA_DIR"): | ||
return os.path.join(os.environ.get("IPP_DATA_DIR"), filename) | ||
return os.path.join(os.path.dirname(__file__), 'data', filename) | ||
|
||
|
||
|
@@ -115,6 +118,12 @@ def handle_www(self): | |
status=200, content_type='text/plain' | ||
) | ||
self.wfile.write(self.server.behaviour.ppd.text()) | ||
elif self.path.endswith('.png'): | ||
self.send_headers( | ||
status=200, content_type='image/png' | ||
) | ||
with open(local_file_location(self.path), 'rb') as wwwfile: | ||
self.wfile.write(wwwfile.read()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do something here to prevent self.path being some horror like |
||
else: | ||
self.send_headers( | ||
status=404, content_type='text/plain' | ||
|
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.
I don't like the mix of
argparse
andos.environ
, as it gives multiple ways to configure things at the same time.But I can't think of another way to do this. (I think the "argparse kool aid" way would be to have a classmethod on the Behaviour which returns the subparser it would like? That feels over-complicated.)
So there's no good reason not do do it like this. Especially as the impact of forgetting to set
IPP_PRINTER_NAME
is very low.