forked from scenic-views/scenic
-
Notifications
You must be signed in to change notification settings - Fork 0
Fetch from Upstream #1
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
Open
tcannonfodder
wants to merge
101
commits into
cheerful:master
Choose a base branch
from
scenic-views:main
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ci skip]
Fix mysql adapter link in README.md.
Maybe it's time to drop support for those versions of Ruby that are no longer being updated? 2.1 is currently end of life, and 2.2 is going to be end of life'd at the end of March.
As of 31/3/2018, Ruby 2.2 and lower are officially unsupported. Also, Rails 4.1 and lower are completely unsupported as well, so I removed those here. Made stuff much cleaner!
Add Ruby 2.5 to travis build matrix
By default, creating a materialized view causes the associated query to be run immediately. The `create view` statement does not complete until the view is populated. While this is a good default, it may be particularly expensive and potentially not desired during deploys. Specifying `no_data: true`, allows the view to be created without a refresh. It will not be usable until it is manually refreshed.
…umper-output Remove extra space from schema dumper output
`pg` 0.21 + has deprecated PGConn in favor of `PG::Connection`. This change fixes the following deprecation: > The PGconn, PGresult, and PGError constants are deprecated, and will be removed as of version 1.0
When we moved to our own organization from the thoughtbot org, we lost the hound-provided rubocop config associated with the thoughtbot org. This change updates to the prefered hound config format that references our own `.rubocop.yml` file. How did I settle on the rubocop rules? Well, I ran rubocop on the codebase and either fixed what came up or added exceptions. We have lots of line length violations that I didn't fix. For the most part, I like the encouragement to stay at 80 characters, but sometimes it's not worth it. I think getting the hound reminder in the PR is sufficient enough for us to have a conversation.
Ruby 2.6 is out, so I've added it to the matrix. Rails 6 (rails edge) requires 2.5+ so we can skip building it with older rubies.
Addresses #258 - cascade failing to identify dependent views when the `view_to_refresh` is a substring of a dependency.
Some folks add `;` to the end of their view definitions to terminate the statement. When this is combined with `no_data: true`, an exception was being raised because `WITH NO DATA` was being interprested as a separate SQL statement. To fix this, we remove the trailing semicolon from the SQL definition before adding `WITH NO DATA`. Fixes #272
Our existing build matrix was a little out of date. We were missing Rails 6 and Ruby 2.7. I could have just added those and called it a day, but our build matrix was already getting unwieldy. I started by removing Ruby 2.3, which is no longer receiving updates. I then removed Rails versions older than 5.2 for the same reason. Finally, I removed the Rubies between 2.4 and 2.7 on the grounds that testing on the oldest currently-supported ruby and the newest is sufficient. Scenic's Rails and Ruby version support remains unchanged, but we're limiting CI to a reasonable combination based on ongoing support from Ruby and Rails. We're saving build time and CO2 emissions.
Prior to this change, `create_view :greetings, version: 2` would error because it was reversed to `drop_view :greetings, version: 2`. The `drop_view` method in our schema statements does not take a `version` argument because that wouldn't make sense. This change removes the `version` argument while leaving others (i.e. `materialized`).
Previous to this change, using the cascading refresh behavior did not also pass through the option to concurrently refresh as one would expect. This meant that while the named view would be refreshed concurrently when desired, none of its dependencies would. Fixes #276
Semantically, I agree that the changelog for a release is the list of every commit and that what we typically refer to as a changelog these days is really a list of newsworthy changes. However, it seems to me that battle is lost and that people expect this file to be named `CHANGELOG.md`. So here it is.
REFRESH MATERIALIZED VIEW CONCURRENTLY doesn't work for a view which is not populated, even it has unique index. This method is useful to perform concurrent refresh if it's possible. Real use case in Rails will be like this: * Database is loaded from structure.sql to run specs, but the view isn't populated yet so concurrent refresh is not possible. We want to fallback to normal(non-concurrent) refresh in this case. * After initial population, views are expected to be refreshed concurrently. It's possible because the view is already populated. Co-authored By: Derek Prior <derekprior@gmail.com>
We had been using Hound CI and a custom RuboCop config to encourage adhering to our style guide. In the last couple of years, Standard has emerged and gained steam as a "set it and forget it" RuboCop configuration for Ruby. Maintaining our own opinionated RuboCop configuration is not something I care to do any longer. This change: 1. Adds Standard 2. Fixes all[^1] off the issues it detects 3. Adds Standard to CI 4. Removes our hound and RuboCop configuration Adding Standard as an explicit CI step will require compliance for passing builds, which is a departure from our previous setup of only commenting via Hound. I think the time is right for this change given the maturity of auto-fixers and integrations within various Ruby workflows. [^1]: We continue to allow the use of `eval` in our specs
Upcoming changes like [topological sorting][1] have the potential to write views to `db/schema.rb` in an order that does not respect dependencies. This adds a test to ensure that views in all schemas are both included, and in the correct dependency order. [1]: #398
This reverts commit d0f2052. It is failing every build on main.
In order to use the model to `find` individual records, the primary key must be explicitly defined. At least that's my experience with PostgreSQL. Also, see https://arjanvandergaag.nl/blog/view-backed-activerecord-models.html
- Reformat FAQ sections to be H3 to improve hierarchy ans so that they can be linked to directly - Add several known blog posts about Scenic
Also remove ~>0.19 constraint on pg gem, which conflicts with ~>1.1 constraint in Gemfile
Upcoming changes like [topological sorting][1] have the potential to write views to `db/schema.rb` in an order that does not respect dependencies. This adds a test to ensure that views in all schemas are both included, and in the correct dependency order. [1]: #398
Co-authored-by: Mykhaylo Sorochan <mykhaylo.sorochan@toptal.com> Co-authored-by: Edward Loveall <edward@edwardloveall.com>
- Test newest Ruby + next Rails (main, 3.4) - Text next Ruby + newest Rails (8.0, 3.3) - Test next Ruby + next Rails (main, 3.4) All of these can fail, but it's useful to know.
The following actions use a deprecated Node.js version and will be forced to
run on node20: actions/cache@v3. For more info:
https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
It's time for everyone's favorite game: bump the versions! Ruby 3.4 is out, and Rails 8 has been shipped for some time. We try to keep our CI suite testing currently-supported versions of each, but we don't want a giant matrix that requires many customizations to run successfully either. As a result, this commit: * Adds testing our full matrix with Ruby 3.4 * Drops testing with Ruby 3.1 * Drops testing with Rails 7.1 I think this is a good balance of testing the latest versions of Ruby and Rails while maintaining some backward compatibility in a manner that doesn't require a degree in YAML to understand.
This is Postgres-specific code, and it shouldn't have been here. #416 (comment)
When performing the sort logic, we were treating results from postgres like they were already Scenic view objects, which they were not. This change makes sure that we are working with the correct objects. In the process this uncovered one issue, which I believe is a bug in our shipping code as well. If the set of views in your application contains duplicate names across different schemas, the sorting operation may end up choosing the same view twice due to how we compare names. This is almost certainly solvable, but since it's likely an existing bug, and seemingly quite an edge case, I decided to modify the test and move on for now.
This implementation will still error if you ask for a concurrent refresh of a non-populated view if your database does not support concurrent refreshes. I'm happy with this because the user has asked for a thing that we know cannot succeed, even under the right circumstances, and we should tell them that., and we should tell them that. Co-authored-by: Derek Prior <derekprior@gmail.com> Co-authored-by: Caleb Hearth <caleb@calebhearth.com>
This adds a `side_by_side` kwarg to the `update_materialized_view` method, which builds the new view alongside the old one and then atomically swaps them to reduce downtime at the cost of increasing disk usage. It is plumbed through to migrations as a hash value for the `materialized` kwarg of `update_view`.
The initial implementation of side_by_side materialized view creation
worked but had a couple of issues that needed to be resolved and I
wanted to refactor the code for better maintainability.
* We had postgres-specific things in the `Scenic::Index` class, which is
not part of the adapter API. The code was refactored to not rely on
adding the schema name to this object.
* Index migration is different from index reapplication, and it felt
like we were reusing `IndexReapplication` just to get access to the
`SAVEPOINT` functionality in that class. I extracted `IndexCreation`
which is now used by `IndexReapplication` and our new class,
`IndexMigration`.
* Side-by-side logic was moved to a class of its own, `SideBySide`, for
encapsulation purposes.
* Instead of conditionally hashing the view name in the case where the
temporary name overflows the postgres identifier limit, we now always
hash the temporary object names. This just keeps the code simpler and
observed behavior from the outside identical no matter identifier
length. This behavior is tested in the new `TemporaryName` class.
* Removed `rename_materialized_view` from the public API on the adapter,
as I'd like to make sure that's something we want separate from this
before we do something like that.
* Added `connection` to the public adapter UI for ease of use from our
helper objects. Documented as internal use only.
* Require a transaction in order to use `side_by_side`. This prevents
leaving the user in a weird state that would be difficult to recover
from.
* Added `--side-by-side` (and `--side_by_side`) support to the
`scenic:view` generator. Also added `--no-data` as an alias for the
existing `--no_data` while I was at it.
* I added a number of tests for new and previously existing code
throughout, including an acceptance level test for `side_by_side`. Our
test coverage should be much improved.
* Updated README with information on `side_by_side`.
Here's a sample of the output from running a `side_by_side` update:
```
== 20250102191533 UpdateSearchesToVersion3: migrating =========================
-- update_view(:searches, {version: 3, revert_to_version: 2, materialized: {side_by_side: true}})
-> temporary materialized view _scenic_sbs_8a03f467c615b126f59617cc510d2abd41296834 has been created
-> indexes on 'searches' have been renamed to avoid collisions
-> index 'index_searches_on_content' on '_scenic_sbs_8a03f467c615b126f59617cc510d2abd41296834' has been created
-> index 'index_searches_on_user_id' on '_scenic_sbs_8a03f467c615b126f59617cc510d2abd41296834' has been created
-> materialized view searches has been dropped
-> temporary materialized view _scenic_sbs_8a03f467c615b126f59617cc510d2abd41296834 has been renamed to searches
-> 0.0299s
== 20250102191533 UpdateSearchesToVersion3: migrated (0.0300s) ================
```
…s for all schemas In Rails 8.1, `bin/rails db:schema:dump` now knows how to dump multiple schemas (rails/rails#50020). To ensure we don't dump the same functions / triggers for each schema, we need to stop memoizing `Scenic.database.views`. This should be fully backwards compatible to previous Rails versions, as we still only call each of those methods once per schema dumping invocation, it just now works if schema dumping is invoked multiple times.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.