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.

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

Remove wiki-feed-parser, add logic to get feeds from supabase #3363

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

TueeNguyen
Copy link
Contributor

@TueeNguyen TueeNguyen commented Mar 30, 2022

Issue This PR Addresses

Fixes #2861
Waiting for #3373 to be merged, then update tests
Waiting for #3361 & #3359

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

  • Rip out wiki-feed-parser.js => will need to update tests as well
  • Add SUPABASE_URL & SERVICE_ROLE_KEY to parser in docker-compose.yml
  • Add supabase client

Steps to test the PR

  • pnpm install
  • pnpm services:start parser
  • Check the parser's container log, if feeds are being processed => it's good

How to run e2e test:

  • pnpm services:start
  • Stop the parser container
  • pnpm jest:e2e parser

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Mar 30, 2022

@humphd
Copy link
Contributor

humphd commented Apr 5, 2022

@TueeNguyen you said you'd update this with details on what's blocking it, can you do that? I'd like to see this get in soon.

@TueeNguyen
Copy link
Contributor Author

I'm tweaking the test, the PR should be up soon @humphd

src/api/parser/env.local Outdated Show resolved Hide resolved
src/api/parser/src/utils/supabase.js Outdated Show resolved Hide resolved
src/api/parser/src/utils/supabase.js Show resolved Hide resolved
src/api/parser/env.local Outdated Show resolved Hide resolved
docker/development.yml Outdated Show resolved Hide resolved
src/api/parser/env.local Outdated Show resolved Hide resolved
docker/development.yml Outdated Show resolved Hide resolved
docker/docker-compose.yml Show resolved Hide resolved
src/api/parser/env.local Outdated Show resolved Hide resolved
src/api/parser/src/parser.js Outdated Show resolved Hide resolved
@humphd
Copy link
Contributor

humphd commented Apr 8, 2022

I'm nervous about landing this until after 3.0-alpha happens, since we've never tested on Staging.

@humphd humphd modified the milestones: 3.0-alpha Release, 3.0 Release Apr 8, 2022
docker/development.yml Outdated Show resolved Hide resolved
@TueeNguyen
Copy link
Contributor Author

In our meeting today, we found out that this code could be removed

const updateFeed = async (feedData) => {
let currentFeed;
// If we have an existing feed in the database for this URL, prefer that,
// since it might have updated cache info (e.g., etag).
const existingFeed = await Feed.byUrl(feedData.url);
if (existingFeed) {
// We have a version of this feed in the database already, prefer that
currentFeed = existingFeed;
} else {
// First time we're seeing this feed, add it to the database
const id = await Feed.create(feedData);
currentFeed = await Feed.byId(id);
}
return currentFeed;
};

It creates new Feed object and saves it to Redis, I think we still need this functionality. I can file an issue on this and open a new PR

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

See #3465 for similar code. I copied what you did, then fixed bugs in it.

- Remove wiki-feed-parser, add logic to get feeds from supabase
- Update test to refect on the change
- Fix Supabase configs, remove secrets from env.local
- Use database staging
@AmasiaNalbandian
Copy link
Contributor

This is looking good when I run it!
image

Copy link
Contributor

@JiaHua-Zou JiaHua-Zou left a comment

Choose a reason for hiding this comment

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

Tested on my machine. Looks good!!

@TueeNguyen TueeNguyen merged commit 3af12d5 into Seneca-CDOT:master Apr 13, 2022
@TueeNguyen TueeNguyen mentioned this pull request Apr 14, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update parser to fetch feeds from Supabase
5 participants