| [21:00:39] | * farmBOT has quit (Ping timeout: 268 seconds) |
| [21:01:04] | * farmBOT has joined #farmos |
| [08:25:29] | <mstenta[m]> | Anyone want to go to Brussels and represent farmOS? :-) https://blog.publiccode.net/news/2022/11/10/fosdem-2023-public-code-and-... |
| [08:32:01] | <mstenta[m]> | 📣 calling all farmOS folks in Europe! 😉 |
| [10:47:49] | <mstenta[m]> | The MariaDB test failures are becoming quite annoying |
| [10:48:34] | <mstenta[m]> | I wonder if we should disable parallel test runs on that too, like we did on SQLite3 |
| [12:08:46] | <paul121[m]> | Yes I was thinking the same! It's been bad lately |
| [13:06:26] | <riotmiked[m]> | Would that impact MariaDB compatibility? Or is it just how tests are run? |
| [13:07:18] | <paul121[m]> | riotmiked[m]: Something with how tests are run. We frequently get this error: https://github.com/farmOS/farmOS/actions/runs/3481776011/jobs/5823385185... |
| [13:07:52] | <paul121[m]> | > <li>Failed to connect to your database server. The server reports the following message: <em class="placeholder">SQLSTATE[HY000] [2002] Connection refused</em>.<ul><li>Is the database server running?</li><li>Does the database exist or does the database user have sufficient privileges to create the database?</li |
| [13:08:16] | <riotmiked[m]> | Roger that - I'm using the MariaDB plugin for Home Assistant for backend. |
| [13:08:26] | <paul121[m]> | but mstenta the latest commit on 2.x was an actual test failure.. https://github.com/farmOS/farmOS/actions/runs/3481180636/jobs/5822761947 |
| [13:09:29] | <paul121[m]> | paul121[m]: oh but the tests did pass on pgsql. this seems like an oauth/consumer related failure. hmmm |
| [13:10:42] | <paul121[m]> | I'm going to re-run all of those jobs on the latest commit :-) |
| [13:20:22] | <symbioquine[m]> | <mstenta[m]> "I wonder if we should disable..." <- With SQLite we had a hypothesis about why they were failing when run in parallel - i.e. the DB doesn't really support parallel access |
| [13:20:22] | <symbioquine[m]> | It doesn't seem like that same hypothesis makes sense for Postgres. Do we have a different one? |
| [13:35:58] | <paul121[m]> | > <@symbioquine:matrix.org> With SQLite we had a hypothesis about why they were failing when run in parallel - i.e. the DB doesn't really support parallel access |
| [13:35:58] | <paul121[m]> | > It doesn't seem like that same hypothesis makes sense for Postgres. Do we have a different one? |
| [13:35:58] | <paul121[m]> | you mentioned Postgres, but think we are talking about mariadb here. Likely the same question though! |
| [13:36:00] | <symbioquine[m]> | ah, yep |
| [13:36:01] | <symbioquine[m]> | My bad |
| [13:37:38] | <paul121[m]> | I know I've had to tweak the number of connections for DBs in the past.. could that be relevant? maybe we can run a few tests in parallel, but we're just running too many? |
| [13:37:48] | <paul121[m]> | and postgres has a higher default than mariadb? |
| [13:38:04] | <symbioquine[m]> | Seems like a good hypothesis |
| [13:38:55] | <symbioquine[m]> | Seems like it would be nice to raise the MariaDB connections (if that's the case) to match rather than reducing the test parallelism |
| [13:39:07] | <paul121[m]> | Not sure that explains why this only happens some of the time. I assume phpunit would run the same number of tests in parallel, but maybe it's an issue of how long individual tests take, which could have some variability? |
| [13:39:25] | <paul121[m]> | Are tests always run in the same order? Is it alphabetically? |
| [13:39:35] | <symbioquine[m]> | Yeah, I bet it's a classic race condition scenario :) |
| [13:41:09] | <symbioquine[m]> | Even if every test were perfectly deterministic and enqueued in the same order we wouldn't necessarily expect the same execution timings given modern kernel/cpu architectures. |
| [13:42:06] | <symbioquine[m]> | Lots of other variables like what other processes are associated with which cores, their nice values, and thread affinities |
| [13:43:47] | <symbioquine[m]> | Not to mention we're probably talking about tests running in VMs on servers running which are running loads for lots of other things so it's easy for differences in which tests are IO, memory, or CPU bound to affect their completion times as a function of noisy neighbors on the host. |
| [13:55:58] | <symbioquine[m]> | <mstenta[m]> "Oh even better... YOU opened the..." <- What I'm struggling with related to this issue now is what the expected response would be for some of the relationships... |
| [13:59:09] | <symbioquine[m]> | For example, it's pretty simple if you go to `/api/asset/animal/resource/relationships/owner/related/schema` it's pretty obvious the type of the owner is "user--user" and so the page for the relationship schema should have `.definitions.data.items.$ref="https://my.farmOS.example/api/user/user/resource/schema"` |
| [14:00:42] | <symbioquine[m]> | But on the other hand we have relationships that aren't constrained to a specific entity/bundle like the `parent` relationship. What would we expect the value of `.definitions.data.items.$ref` to be at `/api/asset/animal/resource/relationships/parent/related/schema`? |
| [14:02:49] | <symbioquine[m]> | I also wonder whether the enum of allowed types on the asset schema should list all the `asset--*` types instead of just being missing? |
| [14:06:35] | <symbioquine[m]> | e.g. Something like;... (full message at <https://libera.ems.host/_matrix/media/v3/download/libera.chat/127386cdeb...) |
| [14:07:10] | <paul121[m]> | symbioquine[m]: I believe I opened an issue for this one :-) |
| [14:08:13] | <symbioquine[m]> | In the `jsonapi_schema` project issue queue? |
| [14:08:42] | <symbioquine[m]> | https://www.drupal.org/project/jsonapi_schema/issues/3174991 ? |
| [14:09:46] | <paul121[m]> | yeah I believe that is it |
| [14:09:52] | <symbioquine[m]> | Cool |
| [14:10:09] | <paul121[m]> | because our equipment reference field does do it! |
| [14:10:26] | <symbioquine[m]> | Yeah, it clearly works correctly under some scenarios |
| [14:11:39] | <paul121[m]> | I forget if a single `entity_reference` field can reference different entity types... if so, might be another complication |
| [14:11:50] | <paul121[m]> | and more reason to include it in the `enum`! |
| [14:11:56] | <mstenta[m]> | > but mstenta the latest commit on 2.x was an actual test failure.. https://github.com/farmOS/farmOS/actions/runs/3481180636/jobs/5822761947 |
| [14:11:56] | <mstenta[m]> | doh! i gave up waiting for it and merged because psql passed |
| [14:12:10] | <mstenta[m]> | did you rerun? |
| [14:16:05] | <paul121[m]> | mstenta[m]: yeah, looks like it passed here: https://github.com/farmOS/farmOS/actions/runs/3481708827 |
| [14:16:27] | <mstenta[m]> | ok phew |
| [14:16:28] | <mstenta[m]> | so we're good? |
| [14:17:07] | <paul121[m]> | yes I think so! |
| [14:27:27] | <mstenta[m]> | paul121: ok if I merge this? https://github.com/farmOS/farmOS/pull/594 |
| [14:27:42] | <mstenta[m]> | I adopted your first suggestion, but punted on the other one |
| [14:27:50] | <paul121[m]> | yep! |
| [14:28:16] | <mstenta[m]> | i have ONE more MR i'd love to toss in front of you for consideration if you have a moment :-) |
| [14:28:23] | <paul121[m]> | sure! |
| [14:28:25] | <mstenta[m]> | https://www.drupal.org/project/farm/issues/3316925 |
| [14:29:09] | <mstenta[m]> | it's pretty minor, and maybe unnecessary/arguable... if it doesn't seem like an easy "accepted" then we can punt/close it |
| [14:32:37] | <paul121[m]> | ah, there's a distinction between "extending" and "using" a class.. the issue description says to prevent downstream from extending classes. so I was surprised to see the log query documentation disappear... but sounds like we want to prevent others from *using* these as well |
| [14:33:15] | <mstenta[m]> | oh good distinction |
| [14:33:25] | <mstenta[m]> | yea i suppose i was thinking both |
| [14:33:42] | <mstenta[m]> | i'm on the fence about the log query one... |
| [14:33:57] | <mstenta[m]> | but i felt like it's easier to "hide" it to start and "reveal" it later than vice versa |
| [14:34:36] | <mstenta[m]> | and the `farm_update` one feels like something we may end up changing - as we find more issues with it |
| [14:34:58] | <mstenta[m]> | and the managed role stuff - i still feel like that could become a standalone drupal module! |
| [14:35:47] | <paul121[m]> | mstenta[m]: yeah I kinda wonder how much we even need this helper tbh. but that's a reason to mark it @internal, if we ever want to remove it.. |
| [14:35:56] | <mstenta[m]> | yeaaa |
| [14:36:02] | <mstenta[m]> | exactly |
| [14:36:05] | <mstenta[m]> | having it public from the start means we are on the hook |
| [14:36:31] | <mstenta[m]> | (another thing to remove from `farm_log` module! haha) |
| [14:37:36] | <paul121[m]> | > The log query service is a helper service for building a standard log database |
| [14:37:36] | <paul121[m]> | query. This is primarily used to query for the "latest" log of an asset. |
| [14:37:36] | <paul121[m]> | The asset location and group membership services use this. |
| [14:37:36] | <paul121[m]> | maybe we just need a service for getting an asset's latest log |
| [14:38:16] | <mstenta[m]> | yea |
| [14:39:08] | <mstenta[m]> | (that's also related to the work i started on the circular location and group membership stuff, and the prereq of passing a timestamp into those services) |
| [14:39:26] | <mstenta[m]> | (which i don't foresee getting done before the next release, but hopefully soon) |
| [14:39:44] | <mstenta[m]> | right now i'm just trying to merge all the easy stuff so we can get a new release out soon |
| [14:40:28] | <mstenta[m]> | (of course, i'd also like to include this one if we can: https://github.com/farmOS/farmOS/issues/579) |
| [14:41:27] | <paul121[m]> | > It makes me think that once we start doing this, it's just another thing we need to remember to add to new classes moving forward as well, so we might end up being inconsistent with it. Maybe better to just never start? |
| [14:41:27] | <paul121[m]> | You commented this on the issue. Inconsistency isn't good, but I think including this on farm_update and managed role things is certainly more important & outweighs any inconsistency concern :-)_ |
| [14:42:16] | <mstenta[m]> | yess |
| [14:42:27] | <mstenta[m]> | i was thinking more about OTHER kinds of classes in that comment (eg `Form`) |
| [14:42:45] | <mstenta[m]> | i think we can just apply to these couple and call it good |
| [14:43:24] | <mstenta[m]> | with `Form` and other common class types, it would become more of an ongoing thing to remember |
| [14:43:46] | <mstenta[m]> | and the only reason i even bring that up is because i saw that Drupal core applies it to some of it's form classes |
| [14:44:06] | <mstenta[m]> | i didn't dig into what the reasoning was, or when that would be a good thing... i'm not sure we need to worry about it |
| [14:44:24] | <paul121[m]> | Ah yeah interesting... yeah almost wish we could explicitly mark which ones are public haha. All `Base___` classes haha |
| [14:44:32] | <mstenta[m]> | yeaa |
| [14:44:41] | <mstenta[m]> | oh well, thanks for the RTBC - i'll merge that |
| [14:45:09] | <mstenta[m]> | are there any other PRs/MRs that folks are hoping to get into the next release (ideally ones that are all ready to go and don't require a lot of deep thinking haha) |
| [14:45:18] | <paul121[m]> | are you familiar with php `final` classes? |
| [14:45:32] | <mstenta[m]> | oh yea, that basically means it CAN'T be extended, right? |
| [14:45:34] | <mstenta[m]> | never really used it |
| [14:45:39] | <paul121[m]> | that would be the way to enforce things are not extended. it might make sense to do that for that managed role permissions tbh |
| [14:45:50] | <mstenta[m]> | ah yea interesting |
| [14:46:13] | <mstenta[m]> | tempted to do that on the group location class... don't make this any more complicated lol |
| [14:46:45] | <paul121[m]> | ah, the class can be final, or individual methods/constants: |
| [14:46:46] | <paul121[m]> | > The final keyword prevents child classes from overriding a method or constant by prefixing the definition with final. If the class itself is being defined final then it cannot be extended. |
| [14:46:55] | <paul121[m]> | yeah |
| [14:47:56] | <paul121[m]> | there are probably clever ways to get around it (maybe alter service definitions to use a different class, like what we did for simple oauth?) but we can do our best |
| [15:36:43] | <mstenta[m]> | > Anyone want to go to Brussels and represent farmOS? :-) https://blog.publiccode.net/news/2022/11/10/fosdem-2023-public-code-and-... |
| [15:36:43] | <mstenta[m]> | I might microblog this to Mastodon/Twitter - it would be cool if someone who is in Europe was able to attend! |
| [16:03:44] | <FarmerEd[m]> | Might be no harm to ask in the forum too, a few active users there don't appear on here very often. |
| [16:24:50] | <mstenta[m]> | https://farmos.discourse.group/t/represent-farmos-at-fosdem/1417 |