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

Slick - Add slick flow functions with error handling using try. #949

Merged
merged 8 commits into from
Jan 25, 2025

Conversation

minjibir
Copy link
Contributor

No description provided.

@pjfanning
Copy link
Contributor

My preference would be not to change the 'Slick' object. Could you just create just create a 2nd 'SlickWithTryResult' or something like that?

@minjibir
Copy link
Contributor Author

Absolutely. That's my first implementation, but later settled with this. I will change it.

@minjibir minjibir force-pushed the error-handling-in-slick-connector branch from 7f399c7 to e63253d Compare January 14, 2025 00:33
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

with both new files, they are based on existing files and they should use the source headers from those files - so can you replace the source headers in your new files with

/*
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * license agreements; and to You under the Apache License, version 2.0:
 *
 *   https://www.apache.org/licenses/LICENSE-2.0
 *
 * This file is part of the Apache Pekko project, which was derived from Akka.
 */

/*
 * Copyright (C) since 2016 Lightbend Inc. <https://www.lightbend.com>
 */

@minjibir
Copy link
Contributor Author

@pjfanning file headers updated. Thank you for pointing that out.

@pjfanning pjfanning added this to the 1.1.1 milestone Jan 16, 2025
@pjfanning
Copy link
Contributor

pjfanning commented Jan 17, 2025

the test code doesn't compile in scala 2.12

[error] /home/runner/work/pekko-connectors/pekko-connectors/slick/src/test/scala/docs/scaladsl/SlickWithTryResultSpec.scala:244:29: type mismatch;
[error]  found   : Seq[SlickWithTryResultSpec.this.User]
[error]  required: scala.collection.immutable.Iterable[?]
[error]       val inserted = Source(duplicateUser)
[error]                             ^
[error] /home/runner/work/pekko-connectors/pekko-connectors/slick/src/test/scala/docs/scaladsl/SlickWithTryResultSpec.scala:245:58: missing parameter type
[error]         .via(SlickWithTryResult.flowTryWithPassThrough { user =>
[error]                                                          ^
[error] /home/runner/work/pekko-connectors/pekko-connectors/slick/src/test/scala/docs/scaladsl/SlickWithTryResultSpec.scala:333:29: type mismatch;
[error]  found   : Seq[SlickWithTryResultSpec.this.User]
[error]  required: scala.collection.immutable.Iterable[?]
[error]       val inserted = Source(duplicateUser)
[error]                             ^
[error] /home/runner/work/pekko-connectors/pekko-connectors/slick/src/test/scala/docs/scaladsl/SlickWithTryResultSpec.scala:344:29: type mismatch;
[error]  found   : Seq[SlickWithTryResultSpec.this.User]
[error]  required: scala.collection.immutable.Iterable[?]
[error]       val inserted = Source(records)
[error]                            

@pjfanning
Copy link
Contributor

I have committed some code changes but I'm not sure if they work yet.

@pjfanning
Copy link
Contributor

seeing test failure in insert 40 records into a table (no parallelism) (new Slick with try support test)

@minjibir minjibir requested a review from pjfanning January 25, 2025 23:32
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

thanks - lgtm

@pjfanning pjfanning merged commit 36f8ff9 into apache:main Jan 25, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants