这是indexloc提供的服务，不要输入任何密码
Skip to content

Feat/token list migration #159

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 11 commits into from
Closed

Feat/token list migration #159

wants to merge 11 commits into from

Conversation

@fengtality
Copy link
Contributor

@fengtality fengtality commented Jul 14, 2023

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:

Revised version of #155

Tests performed by the developer:

Tips for QA testing:

@fengtality fengtality requested review from rapcmia and vic-en July 14, 2023 00:07
rapcmia
rapcmia approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@rapcmia rapcmia left a comment

Choose a reason for hiding this comment

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

LGTM

  • Revision of PR155, test latest commit 9451a6d
  • Run yarn build then check conf/lists folder, all ok ✅
    image
  • Deleted conf folder and observe changes on gateway-setup.sh, all ok ✅
    image
  • Ran tests on both local mbp(m1) and Ubuntu22.04 test server

@nikspz nikspz mentioned this pull request Jul 14, 2023
Closed
@rapcmia
Copy link
Contributor

rapcmia commented Jul 14, 2023

PR update for docker build

  • Create docker image for this PR and latest client development branch

Using hummigbot_gateway-compose

  • Use latest commit for deploy_examples repo
  • Setup hummingbot_gateway_compose file then docker compose up -d
    image
    • On the screenshot above, either list or lists is not available
    • Added docker compose file 👀 
      version: "3.9"
  services:
    hummingbot:
      container_name: hummingbot1
      image: hummingbot/hummingbot:dev
      volumes:
        - "./hummingbot_files/conf:/home/hummingbot/conf"
        - "./hummingbot_files/conf/connectors:/home/hummingbot/conf/connectors"
        - "./hummingbot_files/conf/strategies:/home/hummingbot/conf/strategies"
        - "./hummingbot_files/logs:/home/hummingbot/logs"
        - "./hummingbot_files/data:/home/hummingbot/data"
        - "./hummingbot_files/scripts:/home/hummingbot/scripts"
        - "./hummingbot_files/certs:/home/hummingbot/certs"
      # environment:
      #   - CONFIG_PASSWORD=test
      logging:
        driver: "json-file"
        options:
            max-size: "10m"
            max-file: "5"
      tty: true
      stdin_open: true
      network_mode: host
  
    gateway:
      container_name: gateway1
      image: hummingbot/gateway:159
      ports:
        - "15888:15888"
        - "8080:8080"
      volumes:
        - "./gateway_files/conf:/home/gateway/conf"
      #  - "./gateway_files/conf/lists:/home/gateway/conf/lists" 
        - "./gateway_files/logs:/home/gateway/logs"
        - "./gateway_files/db:/home/gateway/db"
        - "./hummingbot_files/certs:/home/gateway/certs"
      # environment:
      #   - GATEWAY_PASSPHRASE=a
      • Added conf/lists on the file for test purpose
      • Change folder permission as well and check conf/lists still empty

Ran tests using gateway-create.sh

  • Follow the prompts and use PR docker image ✅
    image

vic-en
vic-en requested changes Jul 15, 2023
View reviewed changes
Copy link
Collaborator

@vic-en vic-en left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@fengtality
Docker and source will work after those suggestions are commited.

  1. It's advisable to remove the copy_lists function in the gateway-setup.sh files.
  2. Step 3 of docker compose doc for gateway can be removed as well.

I couldn't push commits because the pr was re-created.

package.json
"repository": "https://github.com/hummingbot/gateway",
"scripts": {
"prebuild": "rimraf dist && mkdir dist",
"prebuild": "rimraf dist && mkdir dist && yarn copy-files:token-list",
Copy link
Collaborator

@vic-en vic-en Jul 15, 2023

Choose a reason for hiding this comment

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

Suggested change
"prebuild": "rimraf dist && mkdir dist && yarn copy-files:token-list",
"prebuild": "rimraf dist && mkdir dist",

package.json
"dev:debug": "NODE_ENV=dev DEBUG=* nodemon src/index.ts",
"start": "/bin/bash ./startup.sh",
"copy-files": "copyfiles -a 'src/**/schema/*.json' 'src/templates/*.yml' 'src/templates/lists/*.json' 'test/services/data/**/*.*' dist",
"copy-files:token-list": "mkdir -p conf/lists && copyfiles -f 'src/templates/lists/*.json' conf/lists",
Copy link
Collaborator

@vic-en vic-en Jul 15, 2023

Choose a reason for hiding this comment

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

Suggested change
"copy-files:token-list": "mkdir -p conf/lists && copyfiles -f 'src/templates/lists/*.json' conf/lists",
"copy-files:token-list": "mkdir -p conf/lists && copyfiles -f -s 'src/templates/lists/*.json' conf/lists",

package.json
"lint": "eslint src test test-scripts --format table --fix",
"dev": "nodemon src/index.ts",
"dev:debug": "NODE_ENV=dev DEBUG=* nodemon src/index.ts",
"start": "/bin/bash ./startup.sh",
Copy link
Collaborator

@vic-en vic-en Jul 15, 2023

Choose a reason for hiding this comment

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

Suggested change
"start": "/bin/bash ./startup.sh",
"start": "yarn copy-files:token-list && /bin/bash ./startup.sh",

@fengtality
Copy link
Contributor Author

fengtality commented Jul 15, 2023

Closed in favor of #160

@fengtality fengtality closed this Jul 15, 2023
@nikspz nikspz mentioned this pull request Jul 17, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vic-en vic-en vic-en requested changes

@rapcmia rapcmia rapcmia approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fengtality @rapcmia @vic-en @cardosofede @nikspz