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

Feature: Allow Sushi to work even when there is no data available. #84

Merged
merged 5 commits into from
Apr 14, 2021
Merged

Feature: Allow Sushi to work even when there is no data available. #84

merged 5 commits into from
Apr 14, 2021

Conversation

Sammyjo20
Copy link

@Sammyjo20 Sammyjo20 commented Jan 14, 2021

Hi Caleb 👋

Thank you for Sushi, I really love using it.

I recently ran into a scenario where I was using getRows() to grab data from a third party API, but sometimes the API would return an empty array as its results. Looking into the Sushi trait, it looks like it depends on at least one row of data to be within the array, because it uses that row to work out the column structure.

To get around this, I wrote two new methods that are invoked on the migrate() method. One is createTable and the other is createTableWithNoData. If there happens to be no rows available, this will look for a new protected $columns where you can define your own table structure, all within the model! I've used the same data types that you had in your original migrate() command to ensure compatibility.

I also built a simple test for this too, but I'd love to know your thoughts.

@danharrin
Copy link
Contributor

Hey Sam! Hope you don't mind me jumping in... how about hijacking the existing $schema array? I do the same with my fork of Sushi for Squire v2 :)

https://github.com/squirephp/squire/blob/main/packages/model/src/Model.php#L124-L126

@Sammyjo20
Copy link
Author

Hey Dan, thanks for your comment - great idea! I'll update that now 😊

@calebporzio
Copy link
Owner

@danharrin are you interested in PRing your update to Sushi core?

@Sammyjo20
Copy link
Author

@calebporzio I updated this PR with Dan's schema logic. Do we need a getSchema() method?

@danharrin
Copy link
Contributor

I need this functionality and this is a really neat implementation, +1 for merging.

@MrHaroldA
Copy link

MrHaroldA commented Mar 11, 2021

We need this feature as well, as new users have empty API repositories. Is the merge conflict easily solvable?

@Sammyjo20
Copy link
Author

@calebporzio This PR had some merge conflicts which I have now resolved.

@tim-bec
Copy link

tim-bec commented Apr 13, 2021

@calebporzio would be great to see this feature in the sushi core. We also experience problems with getRow() and empty api respones.

@calebporzio calebporzio merged commit feb57e2 into calebporzio:master Apr 14, 2021
@calebporzio
Copy link
Owner

Great, thanks for the nudge. I agree 👌

@Sammyjo20
Copy link
Author

Thanks for merging @calebporzio 🤘

@tim-bec
Copy link

tim-bec commented Apr 16, 2021

@calebporzio Would it be possible to create a release for composer? <3

@calebporzio
Copy link
Owner

Done, tagged: v2.2.0

@Sammyjo20 Sammyjo20 deleted the feature/define-columns branch July 14, 2021 13:23
@Sammyjo20 Sammyjo20 restored the feature/define-columns branch July 14, 2021 13:23
@Sammyjo20 Sammyjo20 deleted the feature/define-columns branch July 14, 2021 13:23
calebporzio pushed a commit that referenced this pull request Aug 11, 2021
After introducing sushi to our project, our monitoring tool reported the following
error (stacktrace shortened for brevity):

```
PDOException: SQLSTATE[HY000]: General error: 1 table "schmelzkurve_scoring_table_results" already exists
#88 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): PDOStatement::execute
#87 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): Illuminate\Database\Connection::Illuminate\Database\{closure}
#86 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(685): Illuminate\Database\Connection::runQueryCallback
#85 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(652): Illuminate\Database\Connection::run
#84 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(486): Illuminate\Database\Connection::statement
#83 /vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php(109): Illuminate\Database\Schema\Blueprint::build
#82 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(365): Illuminate\Database\Schema\Builder::build
#81 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(228): Illuminate\Database\Schema\Builder::create
#80 /vendor/calebporzio/sushi/src/Sushi.php(142): App\Models\SchmelzkurveScoringTableResult::createTable
#79 /vendor/calebporzio/sushi/src/Sushi.php(89): App\Models\SchmelzkurveScoringTableResult::migrate
#78 /vendor/calebporzio/sushi/src/Sushi.php(45): App\Models\SchmelzkurveScoringTableResult::Sushi\{closure}
#77 /vendor/calebporzio/sushi/src/Sushi.php(66): App\Models\SchmelzkurveScoringTableResult::bootSushi
```

This error can happen in rare circumstances due to a race condition.
Concurrent requests may both see the necessary preconditions for
the table creation, but only one can actually succeed.

My initial attempt at resolving this was to look for a method that
creates the table only if it does not exist (`create table if not exists`),
but Laravel exposes no such method. Thus, I think the only viable solution
is to risk it and catch the resulting error.
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.

5 participants