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: Dev/transform-next aka query2 #46

Merged
merged 58 commits into from Jan 22, 2018

Conversation

Projects
None yet
3 participants
@johan-bjareholt
Copy link
Member

johan-bjareholt commented Oct 1, 2017

Essentially a simple query language for querying buckets and transforming the data server-side which is much easier to read, can sort results and doesn't use the awkward chunking format anymore.

Also moved all query/transform related files to aw_transform instead of aw_core

TODO:

  • More transform unittests
  • aw-server endpoint
  • More query2 unittests
  • Chunking support for query2
  • More syntax error handling
  • datestart/dateend global variable for query_bucket
  • Implement API on aw-server
  • Remove debug prints
  • Documentation on usage and available functions
  • Typechecking on query2 functions (will be added later when moving to aw-analysis)
  • Update WebUI
  • Write the TODO tests
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 1, 2017

Codecov Report

Merging #46 into master will increase coverage by 2.67%.
The diff coverage is 93.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   94.14%   96.81%   +2.67%     
==========================================
  Files          21       25       +4     
  Lines         905      786     -119     
  Branches      136      119      -17     
==========================================
- Hits          852      761      -91     
+ Misses         23       12      -11     
+ Partials       30       13      -17
Impacted Files Coverage Δ
aw_core/dirs.py 100% <ø> (ø) ⬆️
aw_core/__init__.py 100% <ø> (ø) ⬆️
aw_datastore/storages/mongodb.py 100% <100%> (+2.43%) ⬆️
aw_transform/split_url_events.py 100% <100%> (ø)
aw_transform/__init__.py 100% <100%> (ø)
aw_transform/limit_events.py 100% <100%> (ø)
aw_datastore/storages/memory.py 95.38% <100%> (+1.83%) ⬆️
aw_transform/filter_keyvals.py 100% <100%> (ø)
aw_datastore/storages/abstract.py 100% <100%> (ø) ⬆️
aw_datastore/storages/peewee.py 96.64% <100%> (+0.52%) ⬆️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1848e2a...c05aacb. Read the comment docs.

@johan-bjareholt johan-bjareholt changed the title Dev/transform aka query2 Dev/transform-next aka query2 Oct 1, 2017

@johan-bjareholt johan-bjareholt changed the title Dev/transform-next aka query2 WIP: Dev/transform-next aka query2 Oct 1, 2017

@ErikBjare
Copy link
Member

ErikBjare left a comment

Minor stuff I noticed

# TODO: Proper message
raise QueryException("Nothing to assign")
var, rest = _parse_token(var, namespace)
if var[0] is not Variable:

This comment has been minimized.

@ErikBjare

ErikBjare Oct 1, 2017

Member

You probably mean if not isinstance(var[0], Variable), right?

return val, var

def interpret(var, val, namespace, datastore):
if type(var) is Variable:

This comment has been minimized.

@ErikBjare

ErikBjare Oct 1, 2017

Member

Similar to the typecheck before but this works. Still better to use isinstance though.

for event in events:
if "url" in event.data:
url = event.data["url"]
protocol_end = url.find('://')

This comment has been minimized.

@ErikBjare

ErikBjare Oct 8, 2017

Member

You should definitely be using urllib.parse instead.

This comment has been minimized.

@johan-bjareholt
event.data["path"] = url[domain_end:path_end]
event.data["options"] = url[path_end+1:]
parsed_url = urlparse(url)
event.data["protocol"] = parsed_url.scheme

This comment has been minimized.

@ErikBjare

ErikBjare Oct 8, 2017

Member

I'd like to suggest the following instead:

urlp = urlparse(url)
event.data.update({
    "protocol": urlp.scheme,
    "netloc": urlp.netloc if (not urlp.netloc[:4] == "www.") else urlp.netloc[4:],
    "path": urlp.path,
    "query": urlp.query,
    "fragment": urlp.fragment, 
})

As you see I also changed "options" -> "query", "identifier" -> "fragment", and "domain" -> "netloc" as these are better names that more accurately represent what they contain. It also has the added benefit of being almost identical to the urlparse object. (an argument for also changing "protocol" -> "scheme")

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Oct 8, 2017

Amazing work! ⭐️

@wafflebot wafflebot bot added review and removed in progress labels Oct 14, 2017

johan-bjareholt and others added some commits Oct 14, 2017

Noted that a JavaScript equivalent used to exist
Just in case that's of use to someone
@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented Jan 17, 2018

Now with even more coverage!

@johan-bjareholt johan-bjareholt force-pushed the dev/transform-next branch from 902c08c to f4e445a Jan 17, 2018

@johan-bjareholt johan-bjareholt force-pushed the dev/transform-next branch from 20aa008 to a7d04a9 Jan 21, 2018

@ErikBjare
Copy link
Member

ErikBjare left a comment

I've had a hard time looking over it all, but looks good to me! Very happy with how it turned out 😄

@ErikBjare ErikBjare merged commit 21315cf into master Jan 22, 2018

5 of 6 checks passed

codecov/patch 93.06% of diff hit (target 94.14%)
Details
codecov/project 96.81% (+2.67%) compared to 1848e2a
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@wafflebot wafflebot bot removed the review label Jan 22, 2018

@ErikBjare ErikBjare deleted the dev/transform-next branch Jan 22, 2018

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