Add chainquery dependencies for Spee.ch, does not include migrations #593
Labels
No labels
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
dependencies
Epic
good first issue
hacktoberfest
help wanted
icebox
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
Osprey
priority: blocker
priority: high
priority: low
priority: medium
protocol dependent
resilience
Tom's Wishlist
type: bug
type: discussion
type: error handling
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: LBRYCommunity/spee.ch#593
Loading…
Reference in a new issue
No description provided.
Delete branch "add-chainquery"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ready for review, uses chainquery first then falls back to local; this is needed for my next PRs for trending content and metrics.
this is only used for database migrations. Not of use to speech. I would ignore this table.
This table is used for tracking background job statuses internal to chainquery. Most likely not useful for speech.
same thing. this is an internal table of no importance to speech.
@ -0,0 +1,123 @@
const Sequelize = require('sequelize');
const logger = require('winston');
import abnormalClaimTable from './tables/abnormalClaimTable';
Is this useful to you? This is for claims that were not made through the app. Claims that are not structured according to the lbry protocol.
These are timestamps, is it ok to create it as an integer?
This is a boolean that is a
TINYINT(1)
in the database.This is also an enumeration. I see that you set an enumeration above. Should this be listed as well?
Active,Expired,Controlling,Spent,Accepted
are the enumerations.@ -0,0 +18,4 @@
case 'video/mp4':
return 'mp4';
default:
logger.debug('setting unknown file type as file extension jpg');
Is jpg really a good default? Why not blank or unknown?
Also how do you handle nulls? A channel claim is not going to have any of the meta data fields. They will be null in the DB.
boolean
this is unsigned. Does it matter that you treat it as just an integer?
boolean
@ -0,0 +49,4 @@
type: INTEGER,
set() { },
},
address_list: {
This is a JSON array. Not sure if you can specify that here or not.
Generally, I think there are some tables here that are just not needed for speech and are internal tables for Chainquery. They also don't fall under the major version promise of Chainquery. Those are subject to change in the future. Also there are fields that appear to not have mapped over properly. It may be intentional but I flagged them just in case.
@ -0,0 +18,4 @@
case 'video/mp4':
return 'mp4';
default:
logger.debug('setting unknown file type as file extension jpg');
This is the same implementation as existing code in Spee.ch; it's implemented as a generated field.
I may need to revisit this for reusability.
This will work for now, I need to verify that
Sequelize.DATE(6)
is fully compatible first.There's a few things like
INT(1)
that I'm usingINTEGER
for as well that could probably be switched toSequelize.BOOLEAN
. I know there's a number ofBIGINT
as well that I need to adjust.@tiger5226 which fall under
major version promise
? I'm aiming to keep this independent of the implementations on Spee.chI may need to rethink some of generated fields.
Ha, alright! I didn't realize
TINYINT(1)
is exactly the same asBOOL
https://dev.mysql.com/doc/refman/5.7/en/numeric-type-overview.html
It is more about which ones don't. The below tables are used internally by chainquery:
gorpMigrationsTable
applicationStatusTable
jobStatusTable
A lot of what I commented on was probably taken from what was here already. However, I think it was still good to review those things.
@ -0,0 +1,198 @@
const logger = require('winston');
const returnShortId = (claimsArray, longId) => {
Is this the canonical url?
If so this doesn't seem right. You want the claim_name included here. Also the claim_id is a hash not based on the height. Ignore if this is not the case. @kauffj has a preference for how we do these shortened URLs.
@ -0,0 +65,4 @@
getAllChannelClaims: async (channelClaimId) => {
logger.debug(`claim.getAllChannelClaims for ${channelClaimId}`);
return await table.findAll({
you will want to select specific pieces of data. Otherwise you will get all of it no? The metadata is also in this table as columns, json, and as hex. You probably only want 1 of these versions for your query.
@ -0,0 +82,4 @@
logger.debug(`finding claim id for claim ${claimName} from channel ${channelClaimId}`);
return await table.findAll({
where: { name: claimName, publisher_id: channelClaimId },
order: [['id', 'ASC']],
any reason for ordering?
id
doesn't mean much so it may be wasteful to order the results, then again I think this is the default ordering for mysql anyway.@ -0,0 +91,4 @@
case 1:
return result[0].claim_id;
default:
// Does this actually happen??? (from converted code)
This is a good question actually. Technically, I believe this can happen in the blockchain. In the end every claim has a bid state. So you can always have a claim for a name and you can have 100 of them. Only one can be in the
Controlling
state though.@ -0,0 +101,4 @@
validateLongClaimId: async (name, claimId) => {
return await table.findOne({
where: {
name,
why is name here?
@ -0,0 +117,4 @@
where: {
name,
claim_id: {
[sequelize.Op.like]: `${shortId}%`,
This is a very expensive call. Maybe if you really need to use this, I add this feature to Chainquery so it can simply be matched against.
@ -0,0 +134,4 @@
return await table.findAll({
// TODO: Limit 1
where: { name },
order: [['effective_amount', 'DESC'], ['height', 'ASC']],
So not sure we should use this. The
bid_state
=Controlling
is better. This information comes directly from the blockchain.@ -0,0 +158,4 @@
resolveClaim: async (name, claimId) => {
logger.debug(`Claim.resolveClaim: ${name} ${claimId}`);
return table.findAll({
where: { name, claim_id: claimId },
not sure why name is here like this.
@ -0,0 +174,4 @@
logger.debug(`finding outpoint for ${name}#${claimId}`);
return await table.findAll({
where : { name, claim_id: claimId },
I think I get it. You are not specifying name because they are identical. However, the claim table is unique on
claim_id
. So you do not need to ever you both. There is no benefit that I can think of. If there is let me know.@ -8,3 +10,3 @@
*/
const claimData = ({ ip, originalUrl, body, params }, res) => {
const claimData = async ({ ip, originalUrl, body, params }, res) => {
I like the backup for now. Good for resilience for intial cutover.
@ -0,0 +101,4 @@
validateLongClaimId: async (name, claimId) => {
return await table.findOne({
where: {
name,
The old code validates against the name as well as the claim ID.
I kept the existing functionality - I might remove this.
@ -0,0 +65,4 @@
getAllChannelClaims: async (channelClaimId) => {
logger.debug(`claim.getAllChannelClaims for ${channelClaimId}`);
return await table.findAll({
Good call
@ -0,0 +82,4 @@
logger.debug(`finding claim id for claim ${claimName} from channel ${channelClaimId}`);
return await table.findAll({
where: { name: claimName, publisher_id: channelClaimId },
order: [['id', 'ASC']],
Retained from the old query, I'll remove.
@ -8,3 +10,3 @@
*/
const claimData = ({ ip, originalUrl, body, params }, res) => {
const claimData = async ({ ip, originalUrl, body, params }, res) => {
It's also, unfortunately, required since we don't wait for
chainquery
to have the claim before rendering the content page. We have a number of nuances that require the fallback and prevent us from fully dropping tables.@ -7,1 +8,4 @@
module.exports = {
'/:identifier/:claim': { controller: serveByIdentifierAndClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
'/:claim': { controller: serveByClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
};
@daovist Take a look at the action/saga binding I've added.
You can also use
httpContext.set('key', value)
as well ashttpContext.get('key')
in controllers. I'm using this to communicate route information through the stack programmatically.@ -7,1 +8,4 @@
module.exports = {
'/:identifier/:claim': { controller: serveByIdentifierAndClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
'/:claim': { controller: serveByClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
};
Is this for SSR?
@ -7,1 +8,4 @@
module.exports = {
'/:identifier/:claim': { controller: serveByIdentifierAndClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
'/:claim': { controller: serveByClaim, action: Actions.onHandleShowPageUri, saga: Sagas.handleShowPageUri },
};
Some of it is
Not sure what
Date(6)
is but the db stores aDATETIME
DATE(6)
is the equivalent@ -0,0 +146,4 @@
type: ENUM('Active', 'Expired', 'Controlling', 'Spent', 'Accepted'),
set() { },
},
created_at: {
This is an old column. I need to delete this. Not sure why it is in the schema. I will create an issue for it.
just a few comments
@ -0,0 +117,4 @@
where: {
name,
claim_id: {
[sequelize.Op.like]: `${shortId}%`,
It's an old query, these can be adjusted over time
@ -0,0 +1,123 @@
const Sequelize = require('sequelize');
const logger = require('winston');
import abnormalClaimTable from './tables/abnormalClaimTable';
Not at the moment, retained for full functionality if this is to be split out into a separate lib
@ -0,0 +49,4 @@
type: INTEGER,
set() { },
},
address_list: {
Can't, not supported unless using a virtual field. I might add a custom getter method.
@ -0,0 +1,198 @@
const logger = require('winston');
const returnShortId = (claimsArray, longId) => {
This is the existing logic, I believe. I think there are some issues with it, but we're going to handle it through https://github.com/lbryio/spee.ch/issues/184
Looks good! Some great changes in here.
(I didn't follow everything going on with sagas but I can see how to use them in routes)