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

Add ephemery config url and remove hardcoded config file #9155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gconnect
Copy link
Contributor

PR Description

Fixed Issue(s)

fixes #9121

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@gconnect
Copy link
Contributor Author

@rolfyone please review when you have the time.

@@ -125,6 +126,15 @@
}

private static InputStream loadConfigurationFile(final String source) throws IOException {
if (Eth2Network.EPHEMERY.configName().equalsIgnoreCase(source)) {
try {
URL url = new URL("https://ephemery.dev/latest/metadata/config.yaml");

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note test

Invoking
URL.URL
should be avoided because it has been deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

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

@gconnect
Maybe to avoid using deprecated method you could use
URI.create("https://ephemery.dev/latest/metadata/config.yaml").toURL()

@rolfyone
Copy link
Contributor

Thanks for raising the PR, I'll take a look.
I think potentially this change is more involved, but I'll dig into it.

@rolfyone
Copy link
Contributor

I think the latest url has all the numbers right so really, we'd want to clean up a lot of what's currently done in 'ephemery' for updating config, we shouldn't need it any more.
I'm not sure that the location of this PR code is ideal, thats very low level, but it might be the right place - it just seems like because we don't need all the config update logic (from recollection)
Other minor observations like variables need to be final, and not using deprecated functions but I think the bigger problem is probably that we need to do more analysis.
We could potentially un-assign this and get someone else to do it if that sounds like a lot, but I don't really think this PR is what we are looking for.

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.

update ephemery teku config to pull config from internet
3 participants