Update Pathy and URI #100

safareli · 2018-02-16T15:52:35Z

It compiles but tests are failing, I guess I missed something in *Path print parse functions.

garyb · 2018-02-26T16:03:02Z

src/Quasar/Advanced/Paths.purs

@@ -2,7 +2,7 @@
 Copyright 2017 SlamData, Inc.

 Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file_ except in compliance with the License.


find and replace :d

garyb

☝️, plus it looks like there is a compilation failure in the build now.

garyb · 2018-02-27T10:51:30Z

src/Quasar/Advanced/Types.purs


 parseGroup ∷ String → Either String GroupPath
 parseGroup string =
  Str.stripPrefix (Str.Pattern "group:") string
-  # maybe (Left "Incorrect group") pure
+  # note "Incorrect group"


Let's change these "Incorrect " messages to "Could not parse " instead, while we're in here. We don't want a user to end up seeing these messages as they are, as it sounds like they're doing something wrong, when it's not at all something they have control over.

garyb · 2018-02-27T10:52:09Z

src/Quasar/Data/URI.purs

+type QURIRefOptions =      URI.URIRefOptions URI.UserPassInfo QURIHost AbsPath AbsPath AnyPath QQuery URI.Fragment
+
+type QURI =                URI.URI           URI.UserPassInfo QURIHost AbsPath AbsPath         QQuery URI.Fragment
+type QURIOptions =         URI.URIOptions    URI.UserPassInfo QURIHost AbsPath AbsPath         QQuery URI.Fragment


Please don't align things with whitespace 😄

garyb · 2018-02-27T10:55:33Z

src/Quasar/Mount/MongoDB.purs

@@ -54,59 +49,52 @@ instance showAuth ∷ Show Auth where
    "(Auth { path: " <> show path <> ", credentials: " <> show credentials <> " })"

 type Config =
-  { hosts ∷ NonEmpty Array Host
+  { hosts ∷ URI.QURIHost


This is incorrect now; mongo URIs will need a separate options setup / type configuration, since they're allowed to have multiple hosts. The Data.URI.Extra.MultiHostPortPair is for this.

it's. QURIHost
type QURIHost = URI.MultiHostPortPair URI.Host URI.Port

Oh I see, it's the opposite problem then - every other URI should not support multiple hosts 😄 (it only did that before because we couldn't express otherwise with the types).

garyb · 2018-02-27T10:58:46Z

src/Quasar/QuasarF/Interpreter/Internal.purs

+            URI.HierarchicalPartAuth 
+              authority'
+              (Just (bimap (path </> _) (path </> _) relPath))
+        )


This is a bit hard to read, maybe we could extract it out into another where function and use it like toHierPart authority (Just (bimap (path </> _) (path </> _) relPath))

garyb · 2018-02-27T10:59:13Z

src/Quasar/Types.purs

+
+type AnyPath = AbsPath
+type DirPath = Path Abs Dir
+type FilePath = Path Abs File


Did we add these to pathy in the end?

garyb · 2018-02-27T11:00:05Z

src/Quasar/Data/URI.purs

+printScheme = Scheme.print
+
+unsafeSchemaFromString :: String -> URI.Scheme
+unsafeSchemaFromString = Scheme.unsafeFromString


This should be "Scheme" not "Schema" 😄

garyb · 2018-02-27T11:00:53Z

src/Quasar/Data/URI.purs

+qRelativeRef ∷ PrintParse QRelativeRef
+qRelativeRef = { print: RelativeRef.print opts.relativeRef, parser: RelativeRef.parser opts.relativeRef }
+qURIRef ∷ PrintParse QURIRef
+qURIRef = { print: URIRef.print opts.uriRef, parser: URIRef.parser opts.uriRef }


Maybe could define these as purescript-codec definitions instead?

also reuse AbsPath generator from pathy

garyb · 2018-02-28T14:54:51Z

src/Quasar/FS/Resource.purs

 fromJSON parent json
  = Mount <$> Mount.fromJSON parent json
  <|> do
    obj ← decodeJson json
-    name ← obj .? "name"
+    name' ← note "empty name" <<< fromString =<< (obj .? "name")


I'm not sure about this, since there can be a mount at / (which will have no name, understandably). I'll try and concoct a test case.

This is fine actually, it's just for file listings, so everything must indeed have a name.

garyb

Generally looks 👍, just a few comments/ideas. I can help do those if you'd like to continue on updating SD.

garyb · 2018-02-28T14:56:49Z

src/Quasar/Mount/Common/Gen.purs

+    Just a, Just b -> Just $ Both a b
+    Just a, Nothing -> Just $ This a
+    Nothing, Just b -> Just $ That b
+    Nothing, Nothing -> Nothing


Couple'a unicode bits here and in the type, and in some other places in the PR.

garyb · 2018-02-28T14:57:30Z

src/Quasar/Mount/Couchbase.purs

+    ] <> maybe [] (pure <<< Tuple "queryTimeoutSeconds" <<< Just <<< show <<< un Seconds) queryTimeout
+
+fromURI ∷ URI.QAbsoluteURI → Either String Config
+fromURI (URI.AbsoluteURI scheme (URI.HierarchicalPartNoAuth path) query) = 


Could switch this (and any other like it) to a case _ of while we're here perhaps?

garyb · 2018-02-28T14:58:35Z

src/Quasar/Mount/Mimir.purs

-  = maybe (Left "Couldn't sandbox") Right
-  <<< parseDirPath
+  = note "Couldn't parse absolute dir path"
+  <<< parseQDirPath
  <=< (_ .? "connectionUri")
  <=< (_ .? "mimir")
  <=< decodeJson


Most of these could be changed to just purescript-codec-argonaut JsonCodecs now we have the dependency too, actually.

safareli · 2018-02-28T15:50:28Z

There are quite a good amount of things which need to be update in SD, so let's do
JsonCodec changes in different pr :D

Out of date

garyb

The build is failing at the moment, but looks like it's probably just a test that needs updating.

garyb · 2018-03-15T11:03:07Z

src/Quasar/Mount/Common/Gen.purs

+genAlphaNumericNEString = cons <$> genAlphaNumericChar <*> SG.genString genAlphaNumericChar
+
+genAlphaNumericChar ∷ ∀ m. MonadGen m ⇒ MonadRec m ⇒ m Char
+genAlphaNumericChar = Gen.oneOf $ CG.genDigitChar :| [CG.genAlpha]

 genHostURI ∷ ∀ m. MonadGen m ⇒ MonadRec m ⇒ m URI.Host
 genHostURI = Gen.oneOf $ genIPv4 :| [genName]


There's a URI.Host.Gen.genHost we can use for this now.

garyb · 2018-03-15T11:05:11Z

src/Quasar/Mount/Couchbase.purs

+      (case NES.fromString bucketName of
+        Nothing → Just $ Left P.rootDir
+        Just n → Just $ Right $ P.rootDir </> P.file' (Name n)
+      )


Don't need parens around the case here I think.

garyb · 2018-03-15T15:26:33Z

src/Quasar/Mount/Common/Gen.purs

+    Just a, Just b → Just $ Both a b
+    Just a, Nothing → Just $ This a
+    Nothing, Just b → Just $ That b
+    Nothing, Nothing → Nothing


We can use the one of these I just added to purescript-these now 😄 (sorry)

garyb

But please hold off on merging until #103 is sorted out

garyb · 2018-03-16T11:46:57Z

Oh, I guess we need to merge/release the dependencies too.

safareli · 2018-03-16T13:34:34Z

We should wait until pr in SD passes

safareli added 3 commits February 16, 2018 16:38

compiles

d9c48f5

add final empty segment for dir pathes

0ff247f

fix warnings in tests

794fe13

safareli requested a review from garyb February 16, 2018 15:53

compiles again

f8d02e0

garyb reviewed Feb 26, 2018

View reviewed changes

test passes

aacdd92

garyb previously requested changes Feb 27, 2018

View reviewed changes

safareli added 11 commits February 27, 2018 15:50

fix warnings

53ac539

Remove pathy types from Quasar.Types

14cf8a2

also reuse AbsPath generator from pathy

Incorrect to Could not parse

7401eed

fix Scheme typo

36422e1

use BasicCodec

3e43b55

remove spacing

74a99fa

small refactorsRemoved some reexports from quasar.uri

fc7e01d

add MongoURI type

8a9d618

Merge branch 'master' into update

189ffbb

fix build

fa53800

update uri

b22bf01

garyb reviewed Feb 28, 2018

View reviewed changes

garyb added 3 commits February 28, 2018 16:18

Update for Data.URI -> URI namespace change

2cf844c

Unicode

9b96b1f

Merge remote-tracking branch 'slamdata/master' into update

d82a4e7

garyb changed the title ~~WIP update Pathy and URI~~ Update Pathy and URI Feb 28, 2018

garyb and others added 3 commits March 1, 2018 14:34

Fix Gen for more NES usage in URI

6015a72

add QURI

74f0d80

add QRelativePart

8231e38

garyb and others added 2 commits March 2, 2018 17:21

Update for path-abempty change

6bae08a

update printQPath type

c3ff0cd

safareli force-pushed the update branch from adc9b1c to c3ff0cd Compare March 9, 2018 15:27

safareli added 4 commits March 9, 2018 20:22

printGroupPath ∷ GroupPath → NonEmptyString

0e41b0c

fix build

07a8fa4

remove / from current and parent dir segments

818acbc

add QURIHost' a non empty version of QURIHost

ffd9668

safareli force-pushed the update branch from 173fee3 to ffd9668 Compare March 14, 2018 14:44

add QURIHost' and use it instead of QURIHost

29ce6f8

safareli force-pushed the update branch from 9d854d3 to 29ce6f8 Compare March 14, 2018 15:24

safareli added 2 commits March 14, 2018 16:36

Maybe NonEmptyString to String

9aeba5a

export {parse,print}* functions used in *options

cb44e1c

garyb reviewed Mar 15, 2018

View reviewed changes

fix test; review changes

9f8196c

garyb reviewed Mar 15, 2018

View reviewed changes

use genThese from Data.These.Gen

5c99c8d

garyb previously approved these changes Mar 16, 2018

View reviewed changes

safareli dismissed garyb’s stale review via 9f2cd69 March 19, 2018 17:14

rename printGroupPath to runGroupPath add printGroupPath and printUserId

d2e8b93

safareli force-pushed the update branch from 9f2cd69 to d2e8b93 Compare March 19, 2018 17:27

safareli added 2 commits March 19, 2018 22:56

Merge branch 'master' into update

7e41ab2

upse published version deps

2033964

jdegoes approved these changes Mar 20, 2018

View reviewed changes

cryogenian approved these changes Mar 20, 2018

View reviewed changes

safareli merged commit 13d03a3 into slamdata:master Mar 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Pathy and URI #100

Update Pathy and URI #100

safareli commented Feb 16, 2018

garyb Feb 26, 2018

safareli Feb 26, 2018

garyb left a comment

garyb Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 27, 2018

safareli Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 27, 2018

garyb Feb 28, 2018

garyb Feb 28, 2018

garyb left a comment

garyb Feb 28, 2018

garyb Feb 28, 2018

garyb Feb 28, 2018

safareli commented Feb 28, 2018

garyb left a comment

garyb Mar 15, 2018

garyb Mar 15, 2018

garyb Mar 15, 2018

garyb left a comment

garyb commented Mar 16, 2018

safareli commented Mar 16, 2018

Update Pathy and URI #100

Update Pathy and URI #100

Conversation

safareli commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

safareli commented Feb 28, 2018

garyb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garyb left a comment

Choose a reason for hiding this comment

garyb commented Mar 16, 2018

safareli commented Mar 16, 2018