oats-center / isoblue

ISOBlue Hardware, Avena software, and Deployment Files
MIT License
20 stars 9 forks source link

Bugfix/oada upload/fix sleep #168

Closed aultac closed 1 year ago

aultac commented 2 years ago

This should fix the sleep issue that causes CPU to peg w/ oada_upload.

abalmos commented 2 years ago

I'm not sure what the CI will do with this branch name. If it breaks, you may need to name it like this.

abalmos commented 2 years ago

@aultac Looks like the image is ready for testing with image: isoblue/oada_upload:bugfix-oada_upload-fix_sleep

aultac commented 2 years ago

Fabio told me what the branch name needed to be, and was quite adamant that it had to have this format specifically (bugifx/service_name/bugfixname) or the CI would not work. I don’t know which of you is right :).

Aaron

On Dec 3, 2021, at 4:46 PM, Andrew Balmos @.***> wrote:

I'm not sure what the CI will do with this branch name. If it breaks, you may need to name it like this https://github.com/oats-center/isoblue-avena/blob/master/CONTRIBUTING.md#feature-branches-and-pull-requests.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oats-center/isoblue-avena/pull/168#issuecomment-985862850, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2KEUFP5X2KTLNSXJVXAUTUPE3DDANCNFSM5JKV5ADQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

abalmos commented 2 years ago

@aultac The format is as described in the contributing document. bug-name can be anything, but I was concerned somewhere I spit on / and that having a second would break things. It appears to have worked though.

aultac commented 2 years ago

@fabiocastiblancor is handling this, the PR was only because he thought it needed a PR in order for the CI to build it. It shouldn’t be merged in until he has tested it this week. He’s been giving me and Prof. Evans a tutorial on how to code a service for Avena, and we started w/ attempting to fixing the bug in oada_upload that caused the CPU to peg.

In response to your question about why those two services were commented out, Fabio says that have to be commented out or the CI times out so we commented them out.

Aaron

On Dec 4, 2021, at 12:41 PM, Andrew Balmos @.***> wrote:

@abalmos approved this pull request.

I don't know this code, but a quick glance though seems okay. I left notes a few things.

This service is not in use anywhere, so I think it is safe to merge when @aultac https://github.com/aultac wants. @aultac https://github.com/aultac can clean up any leftover bugs after live testing. I suggest at least a quick sanity check of the published image on a dev ISOBlue.

In .github/workflows/build-and-push-services.yml https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762446789:

  • - cand

  • - j1939d

    Why?

In services/oada_upload/dev-docker-compose.yml https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762447180:

@@ -0,0 +1,20 @@ +version: '3.1' What is the point of this file?

In services/oada_upload/dev-startup.sh https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762447216:

@@ -0,0 +1,8 @@ +#! /bin/bash + +echo "-----------------------------------------------------" +echo "build: " +yarn run build Wny building in a container at runtime? Should be built in the Dockerfile if a build stage is needed.

In services/oada_upload/package.json https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762447308:

"version": "0.0.1", "description": "Upload Avena time-series datapoints to an OADA store",

  • "main": "src/oada_upload.ts",
  • "exports": "./build/oada_upload.js", "repository": "https://github.com/oats-group/isoblue-avena", "author": "Aaron Neustedter", "contributors": [ "Andrew Balmos @.***>" Should add yourself

In services/oada_upload/package.json https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762447336:

"repository": "https://github.com/oats-group/isoblue-avena", "author": "Aaron Neustedter", "contributors": [ "Andrew Balmos @.***>" ], "license": "MIT", "scripts": {

  • "start": "ts-node src/oada_upload.ts"
  • "oldstart": "ts-node --files src/oada_upload.ts", oldstart?

In services/oada_upload/src/atomic-sleep.d.ts https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762447806:

@@ -0,0 +1 @@ +declare module 'atomic-sleep'; I think a proper type would be something like this. I haven't pulled, but does the code pass typechecks?

⬇️ Suggested change -declare module 'atomic-sleep'; +declare module "atomic-sleep" {

-import { isoblueDataTree } from './trees'; +import { isoblueDataTree } from './trees.js'; .js ?

In services/oada_upload/tsconfig.json https://github.com/oats-center/isoblue-avena/pull/168#discussion_r762448078:

@@ -14,6 +14,6 @@ "forceConsistentCasingInFileNames": true, "allowSyntheticDefaultImports": true },

  • "include": ["src/*/"],
  • "imclude": [ "src/*/" ], typo? I'm not sure the include line is even though -- I believe the default is fine.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/oats-center/isoblue-avena/pull/168#pullrequestreview-823285837, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2KEUGJA4S377M3PRXMOLDUPJHE7ANCNFSM5JKV5ADQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.