Run worker binary #201
Run worker binary #201
Conversation
|
|
||
| /** Bindings for cloud platform components using Google Cloud Platform. * */ | ||
| final class GoogleCloudModule extends AbstractModule { | ||
| final class GoogleCloudModule extends CloudExtensionModule { |
There was a problem hiding this comment.
Now or in the next PR, can we name it GoogleCloudExtensionModule for consistency?
There was a problem hiding this comment.
good point, done
| private final ExtensionContext context; | ||
|
|
||
| GoogleCloudModule(ExtensionContext context) { | ||
| this.context = context; |
There was a problem hiding this comment.
Maybe as this evolves, we can pass the objects provided by the context to another Constructor so we can test directly
GoogleCloudModule(HttpTransport transport, JSONFactor factory) {
this.transport = transport;
this.factory = factory;
}
There was a problem hiding this comment.
I had it that way first actually, I didn't want the params list to get too long as it's manually called, I'm fine with either way though. I'll leave for now and we can always change it later
| @@ -26,7 +26,7 @@ | |||
| import org.slf4j.LoggerFactory; | |||
|
|
|||
| /** RSA-based implementation for {@link KeyPair} creation and encoding. */ | |||
| class RsaSymmetricKeyGenerator implements AsymmetricKeyGenerator { | |||
| public class RsaSymmetricKeyGenerator implements AsymmetricKeyGenerator { | |||
There was a problem hiding this comment.
TODO, provide a public module to bind KeyGenerators
| /** | ||
| * Guice module that must be implemented by Cloud extensions. | ||
| */ | ||
| public class CloudExtensionModule extends AbstractModule { |
There was a problem hiding this comment.
Do we expect subclasses to call super.configure() in their configure() impl?
There was a problem hiding this comment.
whoops, yes, good catch
|
|
||
| @Override | ||
| public <T> T getService(Class<T> type) { | ||
| if (type.equals(HttpTransport.class)) { |
There was a problem hiding this comment.
We can also put objects in a map keyed by type and then just read from the map here. In the future as the list gets longer.
| public <T> T getConfiguration(String key, T defaultValue) { | ||
| // TODO parse flags. Returning hardcoded local+GOOGLE config now for testing. | ||
| if (key.equals("cloud")) { | ||
| return (T) "GOOGLE"; |
There was a problem hiding this comment.
it's temporarily hardcoded til we parse flags so I'll address this then
|
I'll go ahead and submit it since it's late. |
Tested running the worker with the Google cloud extension, with our build/run docker script (in config/gcp/)
Currently the Google cloud extension and Microsoft transfer extension are hardcoded since service loading isn't working yet. I left some TODOs for @seehamrun on service loading the extensions.
Left a TODO for flag parsing as well, currently the worker's ExtensionContext is hardcoded to return local config values
#172