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.

Sign up for GitHub

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

Adds option to YAML load of database.yml so that Psych doesn't complain. #10

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adds option to YAML load of database.yml so that Psych doesn't complain. #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc77fc7
Adds option to YAML load of database.yml so that Psych doesn't complain.
infews Feb 5, 2024
c5e7672
Attempts to fix #9 by explicitly creating a new instance of InternalM…
infews Feb 5, 2024
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
2 changes: 1 addition & 1 deletion lib/test_data/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def after_rails_fixture_load(callable = nil, &blk)
end

def database_yaml
YAMLLoader.load_file(database_yaml_full_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 I thought my changes in that file fixed this issue - but perhaps I missed something.

YAMLLoader is just wrapping YAML, and forcing aliases in the event of it needing to.

https://github.com/testdouble/test_data/blob/main/lib/test_data/yaml_loader.rb#L3-L5

The YAMLLoader.load_file method doesn't take a second argument.

It's still not working on your end?

Copy link
Author

Choose a reason for hiding this comment

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

No. Which is why I added the flag.

Copy link
Contributor

@thewatts thewatts Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry if what I said was a little confusing.

Where you added that flag -> that method doesn't take in a second argument. YAMLLoader is a wrapper around YAML, that attempts to take care of the alias problem.

Were you using the most recent gem release? That doesn't have the YAMLLoader changes in it.

I'm assuming that the maintainer isn't going to cut a new release until full 7.1 support is available (it's a work in progress at the moment).

Copy link
Author

Choose a reason for hiding this comment

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

Def using the gem release.

I've dropped this gem for now b/c I'm using SQLite and doing a full extraction for an alternate DB adapter is beyond what I have time for right now. Will look again later. Thanks!

YAMLLoader.load_file(database_yaml_full_path, aliases: true)
end

def database_name
Expand Down
3 changes: 2 additions & 1 deletion lib/test_data/determines_databases_associated_dump_time.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
module TestData
class DeterminesDatabasesAssociatedDumpTime
def call
if (last_dumped_at = ActiveRecord::InternalMetadata.find_by(key: "test_data:last_dumped_at")&.value)
internal_metadata = ActiveRecord::InternalMetadata.new(ActiveRecord::Base.connection)
if (last_dumped_at = internal_metadata["test_data:last_dumped_at"])
Time.parse(last_dumped_at)
end
rescue ActiveRecord::StatementInvalid
Expand Down