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

Release/1.1.1 #167

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,22 @@ jobs:
PGP_SECRET: ${{ secrets.SONA_PGP_SECRET }}
SONATYPE_USERNAME: ${{ secrets.SONA_USER }}
SONATYPE_PASSWORD: ${{ secrets.SONA_PASS }}
- name: Publish ScalaDoc
run: |
project_version=$(sbt version -Dsbt.log.noformat=true | perl -ne 'print "$1\n" if /info.*(\d+\.\d+\.\d+[^\r\n]*)/' | tail -n 1 | tr -d '\n')
if [[ "${{ github.ref }}" = "refs/tags/${project_version}" ]]
then
sbt "project core" makeSite
echo Publishing Scaladoc
git fetch
git checkout gh-pages
cp -r modules/core/target/site/* .
git config user.name "GitHub Actions"
git config user.email "<>"
git add $project_version
git commit -m "Added Scaladoc for $project_version"
git push origin gh-pages
else
echo "${{ github.ref }} does not match project version $project_version => not publishing"
exit 1
fi
5 changes: 5 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
Version 1.1.1 (2021-08-05)
--------------------------
Fix ScalaDoc publishing (#168)
http4s: pass apikey for schema-list methods (#166)

Version 1.1.0 (2021-07-09)
--------------------------
Run scalafmtCheckAll in github action (#163)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@
*/
package com.snowplowanalytics.iglu.client.resolver.registries

import scala.util.control.NonFatal

import cats.implicits._
import cats.data.EitherT
import cats.effect.Sync

import io.circe.Json

import org.http4s.{EntityDecoder, Header, Headers, Method, Request, Status, Uri}
import org.http4s.circe._
import org.http4s.client.{Client => HttpClient}
import org.http4s.{EntityDecoder, Header, Headers, Request, Uri}

import com.snowplowanalytics.iglu.core.{SchemaKey, SchemaList}
import com.snowplowanalytics.iglu.core.circe.CirceIgluCodecs._
import org.http4s.Method.GET

object Http4sRegistryLookup {

Expand Down Expand Up @@ -57,12 +61,10 @@ object Http4sRegistryLookup {
uri <- EitherT.fromEither[F](toPath(http, key))
headers =
http.apikey.fold[Headers](Headers.empty)(apikey => Headers.of(Header("apikey", apikey)))
request = Request[F](method = GET, uri = uri, headers = headers)
json <- EitherT(Sync[F].attempt(client.expect[Json](request))).leftMap[RegistryError] { e =>
val error = s"Unexpected exception fetching: $e"
RegistryError.RepoFailure(error)
}
} yield json
response =
runRequest[F, Json](client, Request[F](method = Method.GET, uri = uri, headers = headers))
result <- EitherT(response)
} yield result

def httpList[F[_]: Sync](
client: HttpClient[F],
Expand All @@ -73,11 +75,14 @@ object Http4sRegistryLookup {
): EitherT[F, RegistryError, SchemaList] =
for {
uri <- EitherT.fromEither[F](toSubpath(http, vendor, name, model))
sl <- EitherT(Sync[F].attempt(client.expect[SchemaList](uri))).leftMap[RegistryError] { e =>
val error = s"Unexpected exception listing: $e"
RegistryError.RepoFailure(error)
}
} yield sl
headers =
http.apikey.fold[Headers](Headers.empty)(apikey => Headers.of(Header("apikey", apikey)))
response = runRequest[F, SchemaList](
client,
Request[F](method = Method.GET, uri = uri, headers = headers)
)
result <- EitherT(response)
} yield result

def toPath(cxn: Registry.HttpConnection, key: SchemaKey): Either[RegistryError, Uri] =
Uri
Expand All @@ -94,5 +99,44 @@ object Http4sRegistryLookup {
.fromString(s"${cxn.uri.toString.stripSuffix("/")}/schemas/$vendor/$name/jsonschema/$model")
.leftMap(e => RegistryError.ClientFailure(e.message))

def runRequest[F[_]: Sync, A: EntityDecoder[F, *]](
client: HttpClient[F],
req: Request[F]
): F[Either[RegistryError, A]] = {
val responseResult = client.run(req).use[F, Either[RegistryError, A]] {
case Status.Successful(response) =>
response
.as[A]
.map(_.asRight[RegistryError])
.handleError {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is exactly what I just said to do..... but I'm looking at it again and wondering if it was the right thing 🤦‍♂️. Because you already handle the error later on at line 134. And also it's inconsistent, because there are other places where you don't catch errors that might get raised (e.g. response.bodyText.compile.string). I guess you've added an informative error message, so maybe it is better.

Oh also..... I think that methods like handleError are guaranteed to be called with non-fatal exceptions only. So it shouldn't be necessary to pattern match on NonFatal. However, I never found good documentation on this - I think I just read it somewhere in a gitter channel.

I don't suggest changing anything though. This is all good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think there's still a slight difference that would help us to catch trimmed payloads.

On line 134 I catch very IO-ish client exceptions, i.e. network is unavailable, connection pool issue etc. Whereas this handleError is specifically about things like decoding failure due trimming or dropped connection and we'll get different error messages.

Although that's post-rationalisation - I also haven't realised the exceptions is caught.

case NonFatal(exception) =>
RegistryError.ClientFailure(s"Could not decode server response. $exception").asLeft[A]
}
case Status.ClientError(response) if response.status.code == 404 =>
(RegistryError.NotFound: RegistryError).asLeft[A].pure[F]
case Status.ServerError(response) =>
response.bodyText.compile.string.map { body =>
val error = s"Unexpected server response: $body"
RegistryError.RepoFailure(error).asLeft
}
case Status.ClientError(response) =>
response.bodyText.compile.string.map { body =>
val error = s"Unexpected server response: $body"
RegistryError.ClientFailure(error).asLeft
}
case response =>
response.bodyText.compile.string.map { body =>
val error = s"Unexpected response: $body"
RegistryError.ClientFailure(error).asLeft
}
}

responseResult.recover {
case NonFatal(exception) =>
val error = Option(exception.getMessage).getOrElse(exception.toString)
RegistryError.ClientFailure(error).asLeft
}
}

implicit def schemaListDecoder[F[_]: Sync]: EntityDecoder[F, SchemaList] = jsonOf[F, SchemaList]
}