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

ROLLUP-471/fix-deployment #109

Merged
merged 17 commits into from
Oct 30, 2023
Merged

ROLLUP-471/fix-deployment #109

merged 17 commits into from
Oct 30, 2023

Conversation

antomor
Copy link
Collaborator

@antomor antomor commented Oct 20, 2023

What

  • in the dev-ticker, read the config required only
  • avoid looking for the market volume of tokens that are unconditionally valid
  • fix dev-ticker container

Why

  • coingecko doens't provide market volume for RBTC, RDOC, USDRIF
  • dev-ticker wasn't able to perform https requests due to a missing package in the container

@antomor antomor changed the title Rollup 471/fix deployment ROLLUP-471/deployment-FIX Oct 23, 2023
@antomor antomor marked this pull request as ready for review October 23, 2023 16:13
@antomor antomor changed the title ROLLUP-471/deployment-FIX ROLLUP-471/deployment-fix Oct 23, 2023
@antomor
Copy link
Collaborator Author

antomor commented Oct 23, 2023

@franciscotobar the unit/integration test flow is failing because we don't define the zksync env in the GH workflow

@@ -35,7 +35,7 @@ services:
start_period: 15s

dev-ticker:

Choose a reason for hiding this comment

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

I guess that locally we dont need the ZKSYNC_HOME, since the component in the dev-ticker that use it, is not necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you're right we don't need it, but I preferred to include it to avoid confusion about it in the different envs.
If we want to remove it from here, we could just add a comment in the other docker-compose.*.yml files

@antomor antomor changed the title ROLLUP-471/deployment-fix ROLLUP-471/fix-deployment Oct 27, 2023
@antomor antomor marked this pull request as draft October 27, 2023 11:43
@antomor antomor marked this pull request as ready for review October 27, 2023 11:43
@antomor antomor marked this pull request as draft October 27, 2023 12:14
@antomor antomor marked this pull request as ready for review October 27, 2023 12:14
@franciscotobar franciscotobar marked this pull request as draft October 27, 2023 14:09
@franciscotobar franciscotobar marked this pull request as ready for review October 27, 2023 14:09
@franciscotobar franciscotobar marked this pull request as draft October 27, 2023 15:03
@franciscotobar franciscotobar marked this pull request as ready for review October 27, 2023 15:03
@franciscotobar franciscotobar force-pushed the ROLLUP-471/fix-deployment branch from 3d306f6 to ca50bf2 Compare October 27, 2023 15:10
@franciscotobar franciscotobar marked this pull request as draft October 27, 2023 15:10
@franciscotobar franciscotobar marked this pull request as ready for review October 27, 2023 15:10
@franciscotobar franciscotobar force-pushed the ROLLUP-471/fix-deployment branch from ca50bf2 to 622c231 Compare October 27, 2023 17:00
@franciscotobar franciscotobar marked this pull request as draft October 27, 2023 17:00
@franciscotobar franciscotobar marked this pull request as ready for review October 27, 2023 17:00
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

Copy link

@franciscotobar franciscotobar left a comment

Choose a reason for hiding this comment

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

LGTM

@antomor antomor merged commit e2e2236 into main Oct 30, 2023
6 checks passed
@antomor antomor deleted the ROLLUP-471/fix-deployment branch October 30, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants