Skip to content

Faster parsing in Neos #9

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

Open
sirkitree opened this issue Jan 28, 2021 · 11 comments · Fixed by #14
Open

Faster parsing in Neos #9

sirkitree opened this issue Jan 28, 2021 · 11 comments · Fixed by #14
Labels
enhancement New feature or request Middleware Neos

Comments

@sirkitree
Copy link
Contributor

This issue is to discuss options for making the parsing in Neos faster for this project.

Currently it's doing two GET operations, and the second one run multiple times, one for each column (to get the cards).

This has visible lag in-world when processing as currently we ensure that processing halts until that data is returned.

My current idea is that we create a new endpoint in the middleware that, given a project id, would return both the columns and their individual cards and parse the whole load at once instead. This may be a little slower on the back-end, but visibly smoother in Neos.

@sirkitree sirkitree added Middleware Neos enhancement New feature or request labels Jan 28, 2021
@sirkitree
Copy link
Contributor Author

SmolCookie has a blueprint for parsing JSON more directly, found in the VRtualis public folder VRtualis > Prefabs > Blueprints and then the "Simple JSON Parser". I haven't used this yet, but looks fairly minimal.
image

There's also the option of utilizing Bombitman's service which transforms JSON data into a more 'friendly' string output.
https://www.polylogix.studio/api/services/api-to-list//v1

I read through this but there is no LogiX example and I'm not even sure how to use the service...

@ProbablePrime
Copy link

That blueprint appears to only work with String variables in JSON

@mralext20
Copy link

perhaps have the server respond in some format where the colums are described, then some sentinal value, like a bunch of pipes, then the cards.

or intermix the cards with the columns, eg

COLUMN,title,,
CARD,title,description,
CARD,title,description,
COLUMN,title,,
CARD,title,description```

@ProbablePrime
Copy link

I have a CSV parser that handles new line characters. that way you dont need to suffix your lines with ",", I think mine is fast but I haven't measured it.

@Anomalous
Copy link

The JSON parser I wrote a while back that we used on the blockchain visualization project was slow mainly because of all the slot duplication during parsing (I think). Since slot duplication has been recently optimized, it might be more viable now. I'd be happy to work on a modernized version of that if it would be useful. I've been tempted to in the past, but it's always been "collections are just around the corner, better to wait for that update which will include a native JSON parsing node."

(I'd probably be more immediately useful at that than the UI stuff; I've started poking a bit at UIX but it's plain it will take a little time to come up to speed.)

@sirkitree
Copy link
Contributor Author

That would be awesome, thanks!

I'm working on an endpoint that will have the JSON response for you to parse. Here is my proposed format:

{
  "Column Name": {
    "Cards": [
      {
        "id": "1",
        "col_id": "1",
        "note": "text to parse"
      },
      {
        "id": "2",
        "col_id": "1",
        "note": "text to parse"
      }
    ]
  }
}

Please let me know if you need any changes to that, but I'll post a PR for review today.

sirkitree added a commit that referenced this issue Feb 2, 2021
- adds a new endpoint at /colcard/:project_id
- gathers columns and endpoints
- returns JSON response with columns and cards within that column
@sirkitree
Copy link
Contributor Author

@Anomalous attached above is the pull request that adds an endpoint. please review

@Anomalous
Copy link

Anomalous commented Feb 3, 2021

@sirkitree It looks reasonable enough to me! I attempted to run the code to take a look for myself (small learning curve as I've never used node.js before), but am getting a "HttpError: Bad credentials" response. Does the GitHub personal access token need a scope beyond "repos"? I did look through the available scopes but nothing jumped out at me as being relevant.

I also want to note that I have not dug into the existing UI code in Neos; I was wanting to get a handle on UIX basics first before tackling a more complicated thing like the Arello board.

@sirkitree
Copy link
Contributor Author

sirkitree commented Feb 3, 2021

Ah sorry, the main readme has install instructions on how to setup your own key for auth

https://github.com/settings/tokens/new?scopes=repo

@Anomalous
Copy link

Somehow I overlooked the README, that was silly. I actually did copy config.example.js to config.js, generate the access token, and copy the access token into the config file. But it looks like I copied it into config.example.js instead of config.js. Whoops. 😅 Not exactly on top of my game tonight, but it's working fine now.

@sirkitree sirkitree linked a pull request Feb 3, 2021 that will close this issue
@sirkitree
Copy link
Contributor Author

sirkitree commented Feb 3, 2021

I've merged the above pull request and deleted the branch. This means you can get this from master now, just switch your local branch.

Note: merging the PR seems to have closed the issue because I referenced the issue (#9). It should not have. It should only do that when certain key words are detected, such as "fix, fixed, fixes": https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@sirkitree sirkitree reopened this Feb 3, 2021
sirkitree added a commit that referenced this issue Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Middleware Neos
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants