all_tables Fixture includes Views

[mulletboy2]

I can't think of a sensible reason for the default behaviour, but I'm not convinced that one doesn't exist.

The 'all_tables' Fixture includes DBIx::Class::ResultSource::Views. These views aren't tables in the DB, so dump_all_sets fails - which necessitates editing all_tables config each time it's generated. There may be some config option I've missed that can get around this?

Looks like adding a check for table_class DBIx::Class::ResultSource::View in _sets_data_from_sources might fix it?

[jjn1056]

Yes totally ! I seldom use views so have not yet been but by this:). Patch?

Thanks for the report!

John

Sent from my iPhone

On May 23, 2013, at 6:08 AM, mark-mullet notifications@github.com wrote:

I can't think of a sensible reason for the default behaviour, but I'm not convinced that one doesn't exist.

The 'all_tables' Fixture includes DBIx::Class::ResultSource::Views. These views aren't tables in the DB, so dump_all_sets fails - which necessitates editing all_tables config each time it's generated. There may be some config option I've missed that can get around this?

Looks like adding a check for table_class DBIx::Class::ResultSource::View in _sets_data_from_sources might fix it?

—
Reply to this email directly or view it on GitHub.

[mulletboy2]

Thanks John,

Never written a patch before. I'll have a crack at it ASAP

Cheers

Mark

On 05/23/2013 12:26 PM, John Napiorkowski wrote:

Yes totally ! I seldom use views so have not yet been but by this:).
Patch?

Thanks for the report!

John

Sent from my iPhone

On May 23, 2013, at 6:08 AM, mark-mullet notifications@github.com
wrote:

I can't think of a sensible reason for the default behaviour, but
I'm not convinced that one doesn't exist.

The 'all_tables' Fixture includes DBIx::Class::ResultSource::Views.
These views aren't tables in the DB, so dump_all_sets fails - which
necessitates editing all_tables config each time it's generated. There
may be some config option I've missed that can get around this?

Looks like adding a check for table_class
DBIx::Class::ResultSource::View in _sets_data_from_sources might fix it?

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub
#23 (comment).

[jjn1056]

So I did a bity of digging here...

If you look here:

https://metacpan.org/source/JJNAPIORK/DBIx-Class-Migration-0.037/lib/DBIx/Class/Migration.pm#L304

that is the guts of how I generate the source list for 'all_tables' What we'd like is something like

my @real_tables = $self->filter_view(@sources)

and then pass @real_tables to the next line when I call the all tables generation stuff.

I think that since all view inherit from

https://metacpan.org/module/DBIx::Class::ResultSource::View

you might be able to do like

my @real_tables = grep { ref($self->schema->source($_)) !~ m/View$/ } @sources;

or something like that. Why not give it a go and let me know?

John

[mulletboy2]

Thanks John,

I've been trying desperately to get back to this and another issue, but
had numerous upgrades to do instead.

I'm off on holiday next week, so if I don't get a chance before, I'm
sure I'll get a chance then. I'll let you know how I get on

Cheers

Mark

On 05/30/2013 08:25 PM, John Napiorkowski wrote:

So I did a bity of digging here...

If you look here:

https://metacpan.org/source/JJNAPIORK/DBIx-Class-Migration-0.037/lib/DBIx/Class/Migration.pm#L304

that is the guts of how I generate the source list for 'all_tables'
What we'd like is something like

my @real_tables = $self->filter_view(@sources
https://github.com/sources)

and then pass @real_tables to the next line when I call the all tables
generation stuff.

I think that since all view inherit from

https://metacpan.org/module/DBIx::Class::ResultSource::View

you might be able to do like

my @real_tables = grep { ref($self->schema->source($_)) !~ m/View$/ }
@sources https://github.com/sources;

or something like that. Why not give it a go and let me know?

John

—
Reply to this email directly or view it on GitHub
#23 (comment).

[jjn1056]

Thanks and look to see some of your other patch on CPAN soon --john

---

From: mark-mullet notifications@github.com
To: jjn1056/DBIx-Class-Migration DBIx-Class-Migration@noreply.github.com
Cc: John Napiorkowski jjn1056@yahoo.com
Sent: Thursday, May 30, 2013 3:29 PM
Subject: Re: [DBIx-Class-Migration] all_tables Fixture includes Views (#23)

Thanks John,

I've been trying desperately to get back to this and another issue, but
had numerous upgrades to do instead.

I'm off on holiday next week, so if I don't get a chance before, I'm
sure I'll get a chance then. I'll let you know how I get on

Cheers

Mark

On 05/30/2013 08:25 PM, John Napiorkowski wrote:

So I did a bity of digging here...

If you look here:

https://metacpan.org/source/JJNAPIORK/DBIx-Class-Migration-0.037/lib/DBIx/Class/Migration.pm#L304

that is the guts of how I generate the source list for 'all_tables'
What we'd like is something like

my @real_tables = $self->filter_view(@sources
https://github.com/sources)

and then pass @real_tables to the next line when I call the all tables
generation stuff.

I think that since all view inherit from

https://metacpan.org/module/DBIx::Class::ResultSource::View

you might be able to do like

my @real_tables = grep { ref($self->schema->source($_)) !~ m/View$/ }
@sources https://github.com/sources;

or something like that. Why not give it a go and let me know?

John

—
Reply to this email directly or view it on GitHub
#23 (comment).

—
Reply to this email directly or view it on GitHub.

[jjn1056]

Hey, sorry I just saw this. Give me a bit to get it out there!

[jjn1056]

hmm. so we skip if the name starts with View... Not sure that's going to roll, you never know what people will call their sources. Lets see if there's a better way to inspect a source to see if its a view or not... I;ll ask around as well

[mulletboy2]

No worries John. Yeah, I did that based on what you said above, and also it seemed in keeping with other code within the module, IIRC. We could compare it with exactly 'DBIx::Class::ResultSource::View'. There was an is_virtual accessor, IIRC, too - but that looked like it was a bad route to take, despite the name.

If there's any way I can assist further, give me a shout.

Cheers

[jjn1056]

Well, I'm really of two minds here :( But I think to be on the safe side ( less chance to bust other people's existing code) I can't see a downside to using the ISA DBIx::Class::ResultSource::View approach. How about you? --john

[mulletboy2]

You're much more familiar with Fixtures than I, but as I understand it, I can't see any sensible use-case for dumping fixtures for views. You can't repopulate the DB with them, and they're only likely to cause errors if you try (which will happen the moment you try to populate the DB). If fixtures were for dumping DB data and not repopulating, I think there'd be a decent argument for keeping the ability to dump views too.

If the above is true, I don't see any way that the ISA DBIx::Class::ResultSource::View approach could be problematic for anyone - they must be doing the same as me, and adding a step of modifying the all-tables fixtures config and removing any views, in order for their fixture population steps to work.

Without knowing other use-cases, it's a tough call. If you don't feel it's worth the risk of adding, I won't be offened. It's not that big a deal for me :)

[jjn1056]

Mark,

Lets do it!  Lets just be sure to mark the changes line in Changes as a 'Tiny Chance of Breaking Code if you are doing weird stuff with views'.

John

On Tuesday, December 17, 2013 10:04 AM, Mark Stringer notifications@github.com wrote:

You're much more familiar with Fixtures than I, but as I understand it, I can't see any sensible use-case for dumping fixtures for views. You can't repopulate the DB with them, and they're only likely to cause errors if you try (which will happen the moment you try to populate the DB). If fixtures were for dumping DB data and not repopulating, I think there'd be a decent argument for keeping the ability to dump views too.
If the above is true, I don't see any way that the ISA DBIx::Class::ResultSource::View approach could be problematic for anyone - they must be doing the same as me, and adding a step of modifying the all-tables fixtures config and removing any views, in order for their fixture population steps to work.
Without knowing other use-cases, it's a tough call. If you don't feel it's worth the risk of adding, I won't be offened. It's not that big a deal for me :)
—
Reply to this email directly or view it on GitHub.

[mohawk2]

@mulletboy2 I see you have an open PR against your own repo. Could you possibly rebase your change against current master (which may have changed in the five years since) and open a PR against this repo for wider review? Thanks!

[mohawk2]

After very little time (in the grand scheme of things), I have merged this to master! Closing.