Skip to content

remove ape#716

Merged
dpaiton merged 6 commits intomainfrom
remove_ape
Jul 22, 2023
Merged

remove ape#716
dpaiton merged 6 commits intomainfrom
remove_ape

Conversation

@wakamex
Copy link
Copy Markdown
Contributor

@wakamex wakamex commented Jul 21, 2023

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
elf-simulations ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2023 1:04am

@github-actions
Copy link
Copy Markdown

Remaining rate limit: 4915 🚀

@wakamex wakamex marked this pull request as draft July 21, 2023 22:22
@wakamex
Copy link
Copy Markdown
Contributor Author

wakamex commented Jul 21, 2023

moved to draft, my intention wasn't to remove ape, just to make the streamlined pipeline work better (just so happens it works better without ape)
keeping as draft for @dpaiton to reference as he works on a more fullsome removal of ape

@github-actions
Copy link
Copy Markdown

Remaining rate limit: 4983 🚀

Comment thread elfpy/bots/bot_info.py
@dpaiton dpaiton marked this pull request as ready for review July 22, 2023 01:10
@dpaiton dpaiton merged commit e83c26b into main Jul 22, 2023
@dpaiton dpaiton deleted the remove_ape branch July 22, 2023 01:15
Copy link
Copy Markdown

@slundqui slundqui left a comment

Choose a reason for hiding this comment

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

Should also look over docker build and various CI workflows for ape. Grep ape should do the trick

# whether to run on devnet
devnet: bool = True
# hostname for container URLs
hostname: str = "http://localhost"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is making the assumption that these services are all on the same machine. Is this always the case?

# hostname for container URLs
hostname: str = "http://localhost"
# ports for container URLs
artifacts_port: str = "80"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why are these not integers?

artifacts_url="http://localhost:80",
rpc_url="http://localhost:8545",
username_register_url="http://localhost:5002",
hostname="http://localhost",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Likewise, same comment. Are these always going to live on the same server?

rpc_url="http://localhost:8545",
username_register_url="http://localhost:5002",
hostname="http://localhost",
artifacts_port="80",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should be int

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.

4 participants