Ah, but hang on tsc is for compiling, not tyêchecking
2020-12-10 34558, 2020
alastairp
so, it's actually failing, and it's got a correct status code, so it should affect the build as we expect it to
2020-12-10 34527, 2020
Mr_Monkey
Cool
2020-12-10 34533, 2020
alastairp
maybe it just tries to compile it
2020-12-10 34534, 2020
Mr_Monkey
As for reporting…
2020-12-10 34542, 2020
alastairp
but --noEmit prevents it from making the .js files
2020-12-10 34554, 2020
alastairp
and so if it fails to build you get the failures?
2020-12-10 34504, 2020
alastairp
(does that mean that there are build errors in that branch?)
2020-12-10 34519, 2020
Mr_Monkey
I do think so
2020-12-10 34500, 2020
alastairp
hey, look at that. tests being useful again!
2020-12-10 34514, 2020
Mr_Monkey
Yeah, at least one of the errors is due to an issue with the js Date types. I'm following the open issue, but I haven't told TS to ignore that error. So it is indeed reported
right, and what does that mean when we actually try to build it?
2020-12-10 34543, 2020
alastairp
will it build despite the fact that it says ERROR ?
2020-12-10 34517, 2020
Mr_Monkey
Yes, webpack should be able to compile it fine (the proof of that being the live version on test.lb)
2020-12-10 34520, 2020
alastairp
kind of makes you wonder what the meaning of 'error' is, then ;)
2020-12-10 34526, 2020
Mr_Monkey
Well, I guess it'll fail the test suite, so there's that… but do remember all that would still be valid javascript, even if it fails dramatically at runtime.
2020-12-10 34506, 2020
Mr_Monkey
But I agree it could be coupled better with webpack, perhaps, so that the webpack build fails on TS errors
2020-12-10 34532, 2020
alastairp
right
2020-12-10 34542, 2020
Mr_Monkey goes to read https://webpack.js.org/guides/typescript/
2020-12-10 34504, 2020
ruaok
alastairp: yep, good to merge.
2020-12-10 34516, 2020
ruaok will continue to be AFK for most of the afternoon
2020-12-10 34554, 2020
alastairp
Mr_Monkey: strangely, no tsc errors here
2020-12-10 34513, 2020
Mr_Monkey
Here on your machine?
2020-12-10 34536, 2020
alastairp
yes
2020-12-10 34558, 2020
alastairp
oh, wait
2020-12-10 34515, 2020
alastairp
this is because I'm on the jenkins branch, which doesn't have playlist code
2020-12-10 34519, 2020
alastairp
ignore me
2020-12-10 34554, 2020
Mr_Monkey ignores alastairp
2020-12-10 34541, 2020
Mr_Monkey
I'm trying locally to add `bail: true` option in webpack config with the goal of failing build in case of typescript errors (without having to run tsc separately).
2020-12-10 34542, 2020
Mr_Monkey
I suppose that's what should happen, instead of reporting errors on one side but allowing build to succeed.
2020-12-10 34514, 2020
alastairp
cool
2020-12-10 34527, 2020
Mr_Monkey
The plot thickens
2020-12-10 34531, 2020
Mr_Monkey
`bail` was a red herring. The `babel-loader` library we use just strips out the typescript instead of validating it.
2020-12-10 34532, 2020
Mr_Monkey
But it looks like with the addition of a plugin to webpack, we can get rid of the `type-checker` container entirely, and do the typescript validation (that will actually fail the build on error) through webpack in a separate thread, with the promise of sped up builds.
that's just a weird thing about the reporting system, I think
2020-12-10 34534, 2020
alastairp
they look that weird on python too :-P
2020-12-10 34548, 2020
alastairp
there's a "new" version of that plugin, that might do it better, but I don't know
2020-12-10 34549, 2020
alastairp
and there is a way of showing the source code and highlighting the error, but there's a configuration issue that I have to fix for that to work, I'll only do it if someone asks for it
[listenbrainz-server] alastair merged pull request #1140 (master…patch-1): Add a note about the spark_reader and link the Spark Architecture document https://github.com/metabrainz/listenbrainz-server…
I don't know if you had any questions, or if you wanted to give it a review? If you think everything is OK then we should set up tests on jenkins and fix these db errors that we were having in CB
2020-12-10 34518, 2020
_lucifer
I don't have questions regarding. The changes are mostly what we had already discussed last month. Plus, your comments explain everything very well :)
2020-12-10 34517, 2020
alastairp
yeah, that's why I did them!
2020-12-10 34542, 2020
alastairp
OK, I'll go ahead with jenkins setup. I guess we need to make some tests for these things that fail in CB, too
2020-12-10 34553, 2020
_lucifer
yeah, makes sense
2020-12-10 34518, 2020
alastairp
then we should decide what to do with CB, should we keep testing the db get methods there, or should we move the tests to BU and mock access in CB?
2020-12-10 34550, 2020
_lucifer
i think the latter one makes more sense
2020-12-10 34559, 2020
_lucifer
have all database tests in BU and then mock it for everything in CB.
2020-12-10 34506, 2020
alastairp
oh, the other thing that I was unsure about was how we should do the db session - should we use a specific pytest module to make sqlalchemy connections, or just do it like I was doing
2020-12-10 34530, 2020
alastairp
access to the MB database in BU is read-only, so I don't think we need any specific testing functionality there
2020-12-10 34526, 2020
alastairp
django has a nice setup where it will run everything in a transaction, which means that resetting after a test is as simple as rolling back the transaction. it's much faster and simpler than our process of dropping tables and recreating them
2020-12-10 34538, 2020
alastairp
but this isn't needed in BU, so I think we shouldn't worry about adding more complexity
2020-12-10 34551, 2020
_lucifer
let's just go ahead with what we already have for now and later use the module if it improves the testing process
[listenbrainz-server] dependabot-preview[bot] opened pull request #1202 (production…dependabot/npm_and_yarn/production/ini-1.3.7): [Security] Bump ini from 1.3.5 to 1.3.7 https://github.com/metabrainz/listenbrainz-server…
[critiquebrainz] dependabot-preview[bot] opened pull request #324 (master…dependabot/npm_and_yarn/ini-1.3.7): [Security] Bump ini from 1.3.5 to 1.3.7 https://github.com/metabrainz/critiquebrainz/pull…
