CORGI Update

[ckline-tryptic]

Description

Since we are retiring Archive and moving fully to GitHub, CORGI needs to change to accommodate this new workflow, and require GitHub authentication. This opportunity is also being taken to update CORGI to V2.

Acceptance Criteria

• CORGI sits behind GitHub authentication.
• Job creation follows the repo batch model by default.
• Jobs can be filtered to identify existing builds.
• Job information is displayed in a cleaner more compact format that conforms to the new workflow.

TODO

• Logging In
  • [X] GitHub auth
  • [X] user icons
• Creating Jobs
  • [X] job_type checkbox (PDF, Web Preview, EPUB, Docx)
  • [X] repo field
  • [X] book field
  • [X] version field
• Job Filtering
  • [X] by repo
  • [X] by book
  • [X] by job_type
• Job Information
  • [X] split collection_id into repo and book
  • [X] add github_user
  • [X] remove style
  • [X] merge pdf_url into status
  • [X] change updated to elapsed
• Tests
  • [ ] frontend
  • [x] backend

Canceled / Deferred

• ~batch jobs~
• ~filter by user~
• ~move rex_preview into details pane~
• ~move worker_version into details pane~

[ckline-tryptic]

The question of whether to migrate CORGI over to Svelte vs sticking with Vue should be resolved sooner rather than later.

Here are a few items for consideration:

Neutral

• Most of the javascript works across both with minimal change (job submission / polling)
• The backend API works across both frameworks and we are making changes to it anyway
• Tests will require changes regardless of framework since we are changing both the DataTable and job submission UI

Pro Svelte

• Svelte is more readable (opinion)

Pro Vue

• The DataTable in Svelte will require a little work to integrate the accordion / responsive functionality (Accordion exists in SvelteMaterialUI, but I need to figure out how to fit it in the Datatable)

[therealmarv]

I also wanted to write down my opinion.

Good and helpful comparison of this two frameworks https://www.vuemastery.com/blog/vue-vs-svelte-comparing-framework-internals/

Pro Svelte

• Restructure of the frontend can be thought of from top to bottom. The current Nuxt frontend has been extended 10 times and nobody (including me) took care of restructuring the code with all functionality in place into separate components and separate the concerns this way. The "large" index.vue is not a Nuxt flaw.
• The Nuxt frontend needs to be upgraded to latest Nuxt 2 version at least (the code only needs to be restructured). EOL of Nuxt 2? (needs research)
• Nuxt 3 (vue.js 3) and Vuetify for Vue.js 3 is ready for production but the Nuxt 3 Auth module (GH authentication) has not been upgraded to Nuxt 3 yet and is a major blocker for Nuxt 3 upgrade https://v3.nuxtjs.org/community/roadmap/#-core-modules
• An upgrade to Nuxt 3 would require rewriting estimated 20-50% of current code.

Con Svelte

• I don't think the Nuxt js code for polling and submission can be reused! Polling happens every 30 seconds and is deeply (UI) framework centric. A complete rewrite of the timed polling and submission code (Svelte best practices? Use Fetch?) is the better approach when switching a framework in my opinion. Using native svelte code (e.g. fetch instead of Axios) will also help the Svelte compiler.
• Vuetify is doing a lot of work in frontend handling for us (very rich UI components like optimised data table, dialogs, dialog checking).
• Major UI richness of Vuetify UI components needs to be rewritten: Data table handling, mobile optimisation, detail view of data table. Reactive good tested data tables are not trivial in my opinion.
• Some minor details need to be rewritten which is mostly taken care of in Vuetify UI components: Auto suggestion, checking of text input data on the fly (on the fly: reg ex, UTF8 compatible type patterns), dialog logic.
• Tests needs to be rewritten.
• UI needs to be manually retested, debugged and optimised on all (corner) cases and rough edges.

Personal opinion:

I don't think the vue code is that much harder to read in comparison to Svelte. It's only more explicit where things live in a vue centric World (e.g. internal value store, events and VirtualDOM update process) and Svelte does some of this things magically in their compiler (that's were the magic in Svelte lies and why it looks like it needs less code). Overall our frontend is not a very big project. What is really missing is better (re)structure and separate things into components and a major rewrite with Svelte can lead to the same results there.

[philschatz]

I haven't had a chance to compare Nuxt & Svelte yet but:

1. How does Nuxt and GitHub Authentication relate? I thought OAuth needed to be done with the server, not a frontend UI.
2. Where are the CORGI mockups again? I forgot to remember the link
3. Where's the link to the svelte code? Ah Here: corgi-svelte . and this is the deployment PR
4. Could Otto pick a test to have Chris rewrite in svelte?

Notes: Unit tests in Jest, Code coverage on all the files, debugging examples (to see how much the auto-injected code and compilation impacts devs)

[m1yag1]

As I was playing around with the UI, I noticed that it might be useful to have a clear button to empty all the fields. It's not super important and mainly for convenience. That's something that I can try and implement.

[ckline-tryptic]

Working on job submission / form validation now.

[TylerZeroMaster]

My opinion:

Svelte Advantages

• Less framework-specific syntax
• The svelte/store library lends itself well to applications architectures like MVVM and MVC
• Even as someone who knows Vue and Vuetify reasonably well, I find svelte easier to use.

  Svelte Disadvantages

• It is easy to misuse svelte's reactivity since the compiler takes a sledgehammer approach (example later)
• Less established than Vue
• Fewer supporting libraries
• Less fleshed out supporting libraries (see the comment @therealmarv had about the data table)
• Will likely require more custom html whereas we use mostly prebuilt vuetify components with vue

Example of sledgehammer reactivity in svelte

If you want to try this yourself, go to svelte.dev and use the editor on the home page.

<script>
    let ticker = false
    let interval = -1
    let timeout = 1000

    $: if (timeout > 0) {
        clearInterval(interval)
        interval = setInterval(() => ticker = !ticker, timeout)
    }
</script>
{@debug interval}
<h1>{ticker ? 'Tick' : 'Tock'}</h1>
Timeout: <input type="number" bind:value={timeout}/>

Problem:

The interval will be cleared and set again every time because ticker is in the scope of the $:, even though it is not directly in scope (it's in an arrow function). The point is that you need to be careful what you include in the scope of one of these $: things because it will run when any global variable in any sub-scope of the $: scope changes.

Explanation

You can see the problem more clearly in the 'compiled js' output:

/* App.svelte generated by Svelte v3.49.0 */
/* -- snip -- */
    $$self.$$.update = () => {
        if ($$self.$$.dirty & /*timeout, interval, ticker*/ 7) {
            $: if (timeout > 0) {
                clearInterval(interval);
                $$invalidate(1, interval = setInterval(() => $$invalidate(0, ticker = !ticker), timeout));
            }
        }
    };
/* -- snip -- */

It shows that the bit that represents ticker is included in the binary AND that is used to determine if an update is required.

Solution

Use a function to remove ticker from the scope.

<script>
    let ticker = false
    let interval = -1
    let timeout = 1000

    function startTicker() {
        interval = setInterval(() => ticker = !ticker, timeout)
    }

    $: if (timeout > 0) {
        clearInterval(interval)
        startTicker()
    }
</script>
{@debug interval}
<h1>{ticker ? 'Tick' : 'Tock'}</h1>
Timeout: <input type="number" bind:value={timeout}/>

[m1yag1]

First, thanks to those before me who submitted such detailed comments. I've been waiting to leave my comments to try and not tilt the balance in any direction, so I'm focusing on the positives.

Pro Svelte

I don't think Svelte will make or break the CORGI frontend. I can see clear advantages to why Svelte is a perfect framework for CORGI. Marvin's referenced article states, "Svelte will shine only when you’re building something like a spreadsheet application where thousands of reactive values are interconnected." I think this statement marks an important point about the strengths of Svelte, which is making reactive sites simple. CORGI is a lot like a spreadsheet application, and it may benefit us to remove all the extra things we don't use in nuxt.js. Removing extra features keeps us from having to maintain them over time. Lastly, the feedback from the community is extremely positive and hard to ignore.

Other pros

• We can serve the javascript code like we deploy our books in s3.

Pro Vue/nuxt.js

As a manager, I must seriously consider why we want to rewrite software. The "secret sauce" of LEAN software development is being purely iterative. By always chasing the next kaizen card, we transform the software we use to what the customer needs. If we didn't determine through experimentation that we need to rewrite the front-end, how do we know we need to rewrite it? Additionally, I have experience working on the front-end, and while there was a small learning curve, I enjoyed the process and felt nuxt.js provided a lot of flexibility we can still leverage into the future. I feel like we could have chosen Svelte or nuxt.js (both released in 2016).

[m1yag1]

No updates to the issue since my last comment (?). It's really hard to tell where this is at w/o digging further.

[therealmarv]

Unfortunately I cannot join the meeting today but I'm biased to Vue personally (partly also because I have written large parts of current CORGI UI). Reasons are mostly that it's an iterative approach as Mike pointed out (we have something running and tested which needs refinement, upgrade and separation of concerns with more components) and Vue can be sprinkled into other possible projects we may have in future. The community is also bigger which is proven e.g. by the more advanced UI framework we currently use.

I think the biggest PRO for Svelte currently is that mainly Chris and partly Tyler are basically owning it and making amazing progress on it. There is currently no bigger effort going on on upgrading our Vue/Nuxt system (I wished I had time for it but there were other high priority things to do). If we continue using Vue it may slow them down in their progress. I definitely do NOT want to slow Chris and Tyler down just because I think that Vue has a little bit more flexibility (no compiler necessarily needed, tooling etc.), bigger community (more libraries like Vuetify) and is more "battle proven" by our users in the last years. I can imagine getting up to speed in a new Svelte CORGI project too.

[ckline-tryptic]

Fixed Icons, added details / actions dialog, fixed elapsed time, hooked up api for buttons. added login redirect. Working on autocomplete.

[TylerZeroMaster]

I finished altering the corgi-concourse-resource so that it will be able to translate to/from the old and new schema. It works locally. Next I would like to see if it will work in a PR environment.

Sidenote:

I ran into some problems because I was working on a branch that was based on a branch that I later squash merged into the main branch of Enki. Incase other people run into this problem, I found what I think is the ideal solution (backup your branch before you try this):

1. Checkout the branch that was based on the one you squash merged
2. Find the last commit that was squash merged into main and copy it's commit sha
3. Run git rebase <commit_sha> --onto main
4. Ideally you should be left with a branch that has all commits after <commit_sha> applied on top of the tip of main (like this one) 🎉.

[TylerZeroMaster]

Last Friday, 10/28/22, we managed to get this working an a PR pipeline here. It works better than expected for the first deploy. Building books works; the status of each job is updated as a book is built; aborting jobs works; and GitHub OAuth mostly works. Honestly, there are only a few problems.

What we know is problematic

• Backend
  1. ~Every instance of the backend uses its own SESSION_SECRET to encrypt session cookies. They should all use the same secret incase there is more than one instance of the backend server running.~ Temporary solution was to create a shared secret in deploy-corgi-pr-stack.
  2. ~Currently the backend allows anyone with a GitHub account to sign in and create a job. We need to re-enable team-based and/or other authorization methods.~
  3. ~Error in corgi_concourse_resource in how it maps web-preview urls.~
  4. ~Cannot add new repos to the database right now.~
  5. Style is an optional property when creating a job, but it is completely ignored during job creation. Style should either be used or removed.
• Frontend
  1. ~Sorting by Book does not work~
  2. ~Sorting by Repo does not work~
  3. ~The Book, Repo, and Version text inputs do not let you enter values that are not in the database.~
  4. The Book field should be a required field until we can update Enki to build multiple books.
  5. ~Elapsed time is wrong because the created_at and updated_at fields in the database do not store timezone information.~
  6. ~Error messages do not work.~
  7. ~IDs should sort DESC by default.~

What should change in the future

• Backend
  1. The PR pipeline should probably use prod-runner instead of dev-runner.
  2. The update job endpoint should only accept lists for artifact_urls. Right now it accepts lists and strings to workaround Enki not supplying book-slug when it puts the pdf_url. (If artifact_urls is a str, it sets the artifact url for the first book in the job).
  3. corgi_concourse_resource is currently acting as a translation layer. It translates back and forth between the new data schema to the one that Enki uses. Ideally, Enki and CORGI would use the same data schema.
  4. Enki needs to be updated to support building multiple books.
  5. CORGI uses the user's token to search for repositories they can access. For the search to find private repos, the oauth app needs full repo access from the user. It might be better to use an organizational token to query repos and then filter by the user's access level.
• Frontend
  1. ~Open artifact links in new tab (target="_blank")~
  2. It would be nice if the Repo input field were wide enough to fit the entire repo name.
  3. There should be feedback or a retry when the user gets a 401.
  4. More feedback about errors. Right now, the user is not notified in any way when there is an error.
• Everything
  1. Unit tests
  2. UI tests
  3. Integration tests
  4. Load tests

     Questions

  5. Several aspects of authentication and authorization are still unclear; what does a good initial release look like?
  6. ...

[omehes]

Initial testing results of the new corgi ui - https://corgi-514.ce.openstax.org/

1. Can the pdf (job artifacts) be open in a separate tab? At the moment it opens in the same tab as the corgi is - I see this is already mentioned

2. Is there enough space to expand the selection fields to maybe see the complete repo and book text (or as much as possible)?

3. webview does not run a job

4. select a repo and book, then delete (clear) book selection and after that clear repo. select a different repo and click the book field. the selection in the book field will show the books for the previously selected repo. I have to refresh browser to make it work correctly again.

5. epub job does not work - getting fetch-utils.ts:11 POST https://corgi-514.ce.openstax.org/api/jobs/ 500

6. I am able to start a job for some repos (not all) while nothing is entered into the book field. Job starts with correct book entry and that book style is added to the book dropdown (even though it was not there before)

7. we used to mark aborted jobs with brown colour. Maybe we could distinguish aborted and failed jobs with colours too (red ! - failed, brown x - aborted)?

8. when all books in the bundle are processed, the resulting link to artifact looks a bit disorganised and only one of the titles is clickable. Maybe they could be listed in separate rows?

[TylerZeroMaster]

I think most of the issues documented here have either been fixed or have a clear path to being fixed. We still need to fix the remaining issues, update all the tests, figure out how authentication and authorization should work in the first release, and get some more feedback from users.

Some feedback that I thought of:

1. When you mouse over Elapsed values, you can see when a job is created. I wonder if this should be reversed. In my opinion, the date/time a job was started is more useful, at a glance, than the elapsed time.
2. ~Error messages need to be shown to the user (maybe a snackbar or a toast). Right now there is literally no indication of errors occurring. You need to use your browser's dev tools to know that an error occurred.~
3. Autocomplete for version field does not work.
4. When the concourse resource checks for new jobs, most of the information it gets from the endpoint is extraneous (i.e. user, books, etc.). It might be a good idea to add something like a jobs-min endpoint that returns the minimum amount of information required for jobs of a specific type.
5. ~Autocomplete does not work correctly for non-openstax repositories.~
6. ~Intermittent 504 Gateway Timeout error on /api/jobs endpoint. When the error occurs, the backend takes > 50 seconds to respond.~ (Something was causing the proxy service to restart at a seemingly random interval. Restarting all the services fixed the issue)

[TylerZeroMaster]

Most of the issues listed previously have been addressed, so I found some more.

1. When you build a repository that was not in the database, the autocomplete list does not update.
2. Webview filtering is broken
3. Some non-book repositories appear in the repo autocomplete list (exmaple template-osbooks)
4. EPUB job type does not exist yet

[TylerZeroMaster]

@omehes

I am able to start a job for some repos (not all) while nothing is entered into the book field. Job starts with correct book entry and that book style is added to the book dropdown (even though it was not there before)

You should be able to build any repo without supplying a book because the backend fetches all the book metadata when you submit a job. Granted, in the initial release, the book field will be. It's possible that an authentication issue interfered with your ability to submit a job. Do you remember which repository you could not build?

we used to mark aborted jobs with brown colour. Maybe we could distinguish aborted and failed jobs with colours too (red ! - failed, brown x - aborted)?

I have done that, but I have not pushed the change to github yet. Same thing with the links.

I think everything else you mentioned has been fixed.

[ckline-tryptic]

Icon colors have been adjusted, and element ids have been added for testing purposes.

[TylerZeroMaster]

Frontend

1. ~When you build a repository that was not in the database, the autocomplete list does not update.~
2. ~Webview filtering is broken~
3. ~EPUB job type does not exist yet (disable checkbox?)~
4. ~Style is an optional property when creating a job, but it is completely ignored during job creation. Style should either be used or removed.~
5. ~The Book field should be a required field until we can update Enki to build multiple books.~
6. ~Expand the selection fields to see the complete repo and book text.~
7. ~Try to figure a way around this strange SMUI autocomplete behavior:~

   select a repo and book, then delete (clear) book selection and after that clear repo. select a different repo and click the book field. the selection in the book field will show the books for the previously selected repo. I have to refresh browser to make it work correctly again.

8. ~More detailed error messages.~

Backend

1. ~Some non-book repositories appear in the repo autocomplete list (example template-osbooks)~
2. ~Style is an optional property when creating a job, but it is completely ignored during job creation. Style should either be used or removed.~
3. ~When the concourse resource checks for new jobs, most of the information it gets from the endpoint is extraneous (i.e. user, books, etc.). It might be a good idea to add something like a jobs-min endpoint that returns the minimum amount of information required for jobs of a specific type.~
4. ~Fix strange behavior when creating jobs. For example, using a commit that exists in a different repository will cause a job to be created for the repository that the commit exists for, regardless of the repository that the user entered.~
5. ~Make sure that everything works correctly for non-admin users.~
6. ~Private repos in autocomplete.~
7. ~More detailed error messages.~
8. Add created/updated at times to new tables.

Waiting for more feedback

1. When you mouse over Elapsed values, you can see when a job is created. I wonder if this should be reversed. In my opinion, the date/time a job was started is more useful, at a glance, than the elapsed time.
2. Understand how filtering broke for at least one book (osbooks-statistics). We have not been able to recreate this issue yet.
   • Filtering should happen on more than one page at a time (reverse the order so that sorting happens before slicing down to page size)

[ckline-tryptic]

Changed status icons to Larissa's.

[TylerZeroMaster]

• Added frontend unit tests and updated EVERYTHING. Surprisingly even though I updated to a different major version of node and updated all of our dependencies, everything continued to work without issue. If anything, it feels like the bundle builds faster than before. Reason for update copy/pasted from commit message:

  We were unknowingly using node version 14 in our frontend image. When I tried to install something using node version 16 (and its npm version) it caused a plethora of problems. We could have sidestepped this issue by installing node dependencies with node version 14; however, there were also several vulnerabilities in the old, outdated package versions we were using. Consequently, it made sense to update node and all of our dependencies.

• The book and repo columns have been swapped as requested (repo before book). There was a small issue where only the headers had been swapped, not the cell contents.
• Added jest to the frontend and created a spec for fetch-utils. We still need additional tests for jobs and utils modules.
• Updated circleci config.yml to run frontend unit tests in node:16-alpine image

[omehes]

Some notes/observations:

1. When incorrect sha or other accepted data is entered into version field, we get a 500 error with not error explanation. Should we add some error text?

2. In the old corgi I used to copy/paste the repo names from the job raws. Don't know how much is this done by other users. It's a bit more difficult to do it in the new corgi UI because the (repeat, approve close) box pops up, then it has to be dismissed and then the highlighted selection can be copied. One word entries can be copy/pasted via simple right-click easier but multi word entries no.

Reply from Tyler: ...that bugs me too. I wonder if it would be better to have a specific clickable region of the row (maybe the ID)

3. New icons: at first sight they look very similar, especially the queued and processing ones. Maybe distinguish them more? also, the jobs which are successful, could the icons be static (like the Abort ones)? That would clearly distinguish them from all the others.

4. I was wondering. Can we handle multi error cases with multi error messages? Like in this case: incorrect book title in Book field and incorrect sha in Version field. Currently, it only shows error for one of them (incorrect sha)...

[TylerZeroMaster]

1. That's interesting, I would expect that to cause a graphql error, but it must be a different error because all graphql errors are forwarded to the client. It looks like GitHub is returning null instead of an error message when an object id that does not exist is used. I added a custom error for this situation. It should be ready for the next deploy.
2. I will start to experiment with this.
3. I agree it’s difficult to differentiate between queued, assigned, and processing. I wonder if it might make them more distinct if we vary the animations for each job status or only animate a subset. I think both the icons and autocomplete functionality could use additional discussion/feedback.
4. Done: it should be ready for the next deploy.

In addition to the above:

• Updated the sorting/filtering to work on more than one page of job
• Made the time in the elapsed tooltip more readable
• Fixed a bug that prevented job creation when the repo field was auto filled by a book selection (additional details below)

  Ottó: I have a situation - steps: click any job type or all select a book in the Book field repo will be automatically filled Create New Job button wont be enabled

[omehes]

Yea, I agree on point 3 above - team discussion (parking for Thursday)

[TylerZeroMaster]

Resolved

1. Handling current data in the database. We need to do something because the current data is incompatible with the new schema. Options:
   1. Data migrations that only run once
      • Problem with using alembic: we would need to guarantee that the data (either old jobs or ABL data) would always be available when that migration runs.
      • Maybe we could run a data migration with a script that exists outside of CORGI
   2. Truncate the data
2. Integration tests
   1. Need to be updated
   2. Need a username and password or an access token to authenticate with. Alternatively, we might be able to shim relevant parts of the GitHub api to get around authentication requirements (but that seems somewhat counter to the idea of integration testing).

Pending

1. Should created_at and updated_at fields be on all tables?
2. How do we start from a clean slate on staging/prod? Is it possible to create a new docker volume and swap them out? That would give us a backup of the current jobs too.

[TylerZeroMaster]

I finished the data migration for the ABL and noticed a small usability problem that we will want to address later. If, for example, you select astronomy as the book to build, the osbooks-astronomy repo is selected automatically, however, the 1e branch is not selected as the version. In this case, it will use main as the version and will not find the old book edition on the main branch because a new edition is on main.

Additionally, there was one error in the ABL data: repo_name: osbooks-college-algebra-bundle sha: ebc5beb15766e5a72d4d5085c1d470ae868007fb In repository: {'slug': 'college-algebra-coreq', 'style': 'precalculus-coreq'} In ABL: {'slug': 'college-algebra-corequisite-support', 'style': 'precalculus-coreq'}

[TylerZeroMaster]

Where we stand as of 22/12/22

I am making one more change to the way that jobs are cached to squeeze out a bit more performance. Other than that, we are ready to merge our changes, release to staging/proc, and wait for the inevitable issues to arise; however, we decided it would be best to wait until after the holidays to deploy.

Before we release, we should, "[...] let Alina know, as she’ll need to send the new documentation to the vendors and let them know it’ll be changing."