Closed lintool closed 8 years ago
Sounds good. This is much more accurately descriptive. I'll refactor?
Let's wait for @ianmilligan1 and @jrwiebe to chime in... but yes, I'll let you do this one.
I approve.
This looks great to me - the more descriptive we can make our commands the better. Please go ahead @aliceranzhou!
:) will do!
On Wed, Nov 25, 2015 at 8:12 AM Ian Milligan notifications@github.com wrote:
This looks great to me - the more descriptive we can make our commands the better. Please go ahead @aliceranzhou https://github.com/aliceranzhou!
— Reply to this email directly or view it on GitHub https://github.com/lintool/warcbase/issues/177#issuecomment-159603110.
Please review? https://github.com/lintool/warcbase/tree/refactor-api
On Wed, Nov 25, 2015 at 1:05 PM Alice Zhou alice.zhou@gmail.com wrote:
:) will do!
On Wed, Nov 25, 2015 at 8:12 AM Ian Milligan notifications@github.com wrote:
This looks great to me - the more descriptive we can make our commands the better. Please go ahead @aliceranzhou https://github.com/aliceranzhou!
— Reply to this email directly or view it on GitHub https://github.com/lintool/warcbase/issues/177#issuecomment-159603110.
Can you send it in the form of a pull request so I can see the diffs easily?
One minor comment. Otherwise, ship it!
As a test for getContentRaw
, do you want to add a new EncodeBase64 UDF and try the trick mentioned in #178 ?
Thanks!
Sure, will do!
On Wed, Nov 25, 2015 at 5:29 PM Jimmy Lin notifications@github.com wrote:
One minor comment. Otherwise, ship it!
As a test for getContentRaw, do you want to add a new EncodeBase64 UDF and try the trick mentioned in #178 https://github.com/lintool/warcbase/issues/178 ?
— Reply to this email directly or view it on GitHub https://github.com/lintool/warcbase/issues/177#issuecomment-159746008.
I've verified that the generated base64 strings decode to valid images. However, Spark Notebook doesn't seem to want to display them.
Also, an interesting discovery: the HTML injection trick only works if the HTML is within an element that will be displayed as a table. A single string will not be displayed as HTML.
Odd. Browser issue? If you pop open chrome js console and examine the DOM element, does it show anything helpful? Maybe some other js is getting in the way?
I assume if you copy the same exact HTML in http://jsfiddle.net/ the image shows up?
Closing. I believe this was merged as part of 8e8f072aca5fd604974f7a1b938e137cd284466b
@aliceranzhou @ianmilligan1 @jrwiebe I'm looking at the API... so, currently we have:
getBodyContent
, which fetches the raw HTML of a pagegetRawBodyContent
, which is basicallyExtractRawText(r.getBodyContent)
, which throws away all HTMLA have a few issues with this...
getRawBodyContent
is misnamed, in that raw suggests the original content, but we actually mean "cleaned up text".If we're going to refactor the API, we should do it sooner rather than later before everyone gets set in their ways...
Here's what I propose: replace the above with
getContentString
which returns original content, which in the case of HTML pages, is the raw HTML. For images, it would return gibberish.getContentBytes
which returns original content as a byte array (i.e., raw bytes) so it can be manipulated further downstreamExtractRawText
toRemoveHTML
(or something along those lines) to make more descriptive.So we would get rid of
getRawBodyContent
and everyone would have to sayRemoveHTML(r.getContentString)
- which is a bit longer but far more accurately descriptive.Thoughts?