You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Vitaly <no...@github.com> on 2013/10/24 16:10:59 UTC
[jclouds-labs] Joyent Blob storage integration (#30)
Joyent blob storage provider implementation for jClouds
You can merge this Pull Request by running:
git pull https://github.com/vitaly-rudenya/jclouds-labs master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/30
-- Commit Summary --
* Initial commit
* Code reformatting
* Code review notes fixes
* Java docs creation
* Internal tolls were used
* Tests were added
* Tests were added
-- File Changes --
A joyentblob/pom.xml (131)
A joyentblob/src/main/java/org/jclouds/joyent/JoyentApiMetadata.java (85)
A joyentblob/src/main/java/org/jclouds/joyent/JoyentBlobAsyncClient.java (157)
A joyentblob/src/main/java/org/jclouds/joyent/JoyentBlobClient.java (84)
A joyentblob/src/main/java/org/jclouds/joyent/JoyentConstants.java (40)
A joyentblob/src/main/java/org/jclouds/joyent/JoyentProviderMetadata.java (86)
A joyentblob/src/main/java/org/jclouds/joyent/blobstore/JoyentAsyncBlobStore.java (149)
A joyentblob/src/main/java/org/jclouds/joyent/blobstore/JoyentBlobRequestSigner.java (138)
A joyentblob/src/main/java/org/jclouds/joyent/blobstore/JoyentBlobStore.java (136)
A joyentblob/src/main/java/org/jclouds/joyent/blobstore/config/JoyentBlobStoreContextModule.java (41)
A joyentblob/src/main/java/org/jclouds/joyent/config/JoyentBlobRestClientModule.java (48)
A joyentblob/src/main/java/org/jclouds/joyent/functions/ListBlobsResponseToResourceList.java (60)
A joyentblob/src/main/java/org/jclouds/joyent/functions/ParseBlobFromHeadersAndHttpContent.java (48)
A joyentblob/src/main/java/org/jclouds/joyent/functions/ParseBlobPropertiesFromHeaders.java (55)
A joyentblob/src/main/java/org/jclouds/joyent/parsers/JoyentBlobMetadataParser.java (108)
A joyentblob/src/main/java/org/jclouds/joyent/parsers/JoyentObject.java (117)
A joyentblob/src/main/java/org/jclouds/joyent/parsers/ParseBlobFromJoyentResponse.java (62)
A joyentblob/src/main/java/org/jclouds/joyent/parsers/ParseBlobMetadataFromJoyentResponse.java (45)
A joyentblob/src/main/java/org/jclouds/joyent/parsers/ParseObjectInfoListFromJoyentResponse.java (81)
A joyentblob/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
A joyentblob/src/test/java/org/jclouds/joyent/JoyentAsyncClientTest.java (123)
A joyentblob/src/test/java/org/jclouds/joyent/JoyentBlobProviderTest.java (31)
A joyentblob/src/test/java/org/jclouds/joyent/JoyentObjectStorageClientLiveTest.java (183)
A joyentblob/src/test/java/org/jclouds/joyent/TestConstants.java (30)
A joyentblob/src/test/resources/data/Master-Yoda.jpg (0)
M pom.xml (1)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/30.patch
https://github.com/jclouds/jclouds-labs/pull/30.diff
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.inject.Inject;
> +import javax.inject.Singleton;
> +
> +/**
> + * Parses response headers and creates a new Blob from them and the HTTP content.
> + *
> + * @author vitaly.rudenya
> + */
> +@Singleton
> +public class ParseBlobFromHeadersAndHttpContent implements Function<HttpResponse, Blob> {
> +
> + private final Blob.Factory blobFactory;
> +
> + @Inject
> + public ParseBlobFromHeadersAndHttpContent(Blob.Factory blobFactory) {
> + this.blobFactory = blobFactory;
checkNotNull here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7260164
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + * Put Blob object into Joyent storage.
> + */
> + public String putBlob(String container, Blob object);
> +
> + /**
> + * Retrieve Blob from Joyent storage
> + */
> + public Blob getBlob(String container, String name);
> +
> + /**
> + * Retrieve Blob metadata from Joyent storage
> + */
> + public BlobMetadata getBlobMetadata(String container, String name);
> +
> + /**
> + * Create new folder in the rood directory.
Same typo as above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7241520
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + ListenableFuture<Blob> getBlob(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Retrieve Blob metadata from Joyent storage
> + */
> + @Named("GetBlobMetadata")
> + @GET
> + @ResponseParser(ParseBlobMetadataFromJoyentResponse.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Path("/{container}/{name}")
> + public ListenableFuture<BlobMetadata> getBlobMetadata(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Create new folder in the rood directory.
I meant, that it should be roo"t", not roo"d"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7242442
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
Closing as there seems to be no activity here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-49412770
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Vitaly <no...@github.com>.
Hello
As I know there some negotiates about using joyent servers during tests while build is running
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-30409542
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + Set<StorageMetadata> storageMetadatas = new HashSet<StorageMetadata>();
> +
> + for (JoyentObject joyentObject : form) {
> + StorageType storageType = null;
> + if (JoyentObject.TYPE_FOLDER.equals(joyentObject.getContentType())) {
> + storageType = StorageType.FOLDER;
> + } else if (JoyentObject.TYPE_BLOB.equals(joyentObject.getContentType())) {
> + storageType = StorageType.BLOB;
> + }
> +
> + Date creationDate = joyentObject.getLastModified();
> + @SuppressWarnings("unchecked") StorageMetadata storageMetadata = new StorageMetadataImpl(storageType, null,
> + joyentObject.getName(), null, null, joyentObject.getEtag(), creationDate, creationDate,
> + Collections.EMPTY_MAP);
> + storageMetadatas.add(storageMetadata);
> + }
If you encapsulate the transformation in a `Function<JoyentObject, StorageMetadata>`, you can remove the loop and take advantage of the Guava's lazy iterators and lazy transformations, to transform only the elements that are actually read.
```java
return new PageSetImpl<StorageMetadata>(Iterables.transform(form, transformationFunction), null);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7242943
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
Closed #30.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#event-143025291
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + @Override
> + public JoyentBlobMetadataParser<RETURN> setContext(HttpRequest request) {
> + if (request instanceof GeneratedHttpRequest) {
> +
> + List args = ((GeneratedHttpRequest) request).getInvocation().getArgs();
> +
> + container = (String) args.get(0);
> + blobName = (String) args.get(1);
> + }
> +
> + String reqURL = request.getEndpoint().toString();
> + String path = request.getEndpoint().getPath();
> + String storPath = "/" + identity + "/" + JoyentConstants.STOR_PATH;
> + if (!path.startsWith(storPath)) {
> + int pathInd = reqURL.lastIndexOf(path);
> + String newUrl = reqURL.substring(0, pathInd) + storPath + reqURL.substring(pathInd);
This same code is also used when signing the requests. Could you extract/refactor it so you have it only in one place?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7260263
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + assertEquals(request.getFilters().get(0).getClass(), JoyentBlobRequestSigner.class);
> + }
> +
> + @Override
> + public JoyentProviderMetadata createProviderMetadata() {
> + return new JoyentProviderMetadata();
> + }
> +
> + protected java.util.Properties setupProperties() {
> + Properties overrides = super.setupProperties();
> + overrides.setProperty(Constants.PROPERTY_IDENTITY, TestConstants.USER_NAME);
> + overrides.setProperty(JoyentConstants.JOYENT_CERT_FINGERPRINT, TestConstants.CERT_FINGERPRINT);
> + overrides.setProperty(JoyentConstants.JOYENT_CERT_CLASSPATH, TestConstants.CERT_CLASSPATH);
> + return overrides;
> + }
> +}
It is great that you added the tests for the Joyent API class, however, this kind of unit tests are deprecated and we no longer tests the APIs this way. Now we use [mockwebserver](https://code.google.com/p/mockwebserver/) for unit testing as it gives us a more complete test path. You can take a look at the OpenStack Swift tests to see how it works (for example you can take a look at the [ContainerApiMockTest](https://github.com/jclouds/jclouds-labs-openstack/blob/master/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ContainerApiMockTest.java) test class).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7243045
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Vitaly <no...@github.com>.
> +
> + return res;
> + }
> +
> + private static KeyPair getKeyPair(String keyPath) throws IOException {
> + BufferedReader br =
> + new BufferedReader(new InputStreamReader(JoyentBlobRequestSigner.class.getResourceAsStream(keyPath)));
> +
> + Security.addProvider(new BouncyCastleProvider());
> + PEMReader pemReader = new PEMReader(br);
> + try {
> + return (KeyPair) pemReader.readObject();
> + } finally {
> + pemReader.close();
> + }
> + }
I have to use SHA256WithRSAEncryption algorithm to sign requests. Using this approach I have NoSuchAlgorithmException error
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7245193
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Vitaly <no...@github.com>.
> + ListenableFuture<Blob> getBlob(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Retrieve Blob metadata from Joyent storage
> + */
> + @Named("GetBlobMetadata")
> + @GET
> + @ResponseParser(ParseBlobMetadataFromJoyentResponse.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Path("/{container}/{name}")
> + public ListenableFuture<BlobMetadata> getBlobMetadata(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Create new folder in the rood directory.
Root is main or "/" folder in this case.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7242270
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Vitaly <no...@github.com>.
> + ListenableFuture<Blob> getBlob(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Retrieve Blob metadata from Joyent storage
> + */
> + @Named("GetBlobMetadata")
> + @GET
> + @ResponseParser(ParseBlobMetadataFromJoyentResponse.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Path("/{container}/{name}")
> + public ListenableFuture<BlobMetadata> getBlobMetadata(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Create new folder in the rood directory.
U-ups :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7242563
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
@vitaly-rudenya @cobracmder It's been two months since the PR was opened. Is there any chance it gets some progress regarding the review comments? (No pressure, we are just trying to filter dead PRs).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-30180902
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + return res;
> + }
> +
> + private static KeyPair getKeyPair(String keyPath) throws IOException {
> + BufferedReader br =
> + new BufferedReader(new InputStreamReader(JoyentBlobRequestSigner.class.getResourceAsStream(keyPath)));
> +
> + Security.addProvider(new BouncyCastleProvider());
> + PEMReader pemReader = new PEMReader(br);
> + try {
> + return (KeyPair) pemReader.readObject();
> + } finally {
> + pemReader.close();
> + }
> + }
Ok, let's stay with Bouncycastle then. Consider, though, if it is worth moving the private key loading to a memoized supplier.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7260082
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
Just for the record, the initial review is here: https://github.com/jclouds/jclouds/pull/188
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-26996115
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +/**
> + * @author vitaly.rudenya
> + */
> +@Singleton
> +public class JoyentBlobStore extends BaseBlobStore {
> +
> + private final JoyentBlobClient sync;
> + private final ListBlobsResponseToResourceList listBlobsResponseToResourceList;
> +
> + @Inject
> + JoyentBlobStore(BlobStoreContext context, BlobUtils blobUtils, Supplier<Location> defaultLocation,
> + @Memoized Supplier<Set<? extends Location>> locations, JoyentBlobClient sync,
> + ListBlobsResponseToResourceList listBlobsResponseToResourceList) {
> + super(context, blobUtils, defaultLocation, locations);
> + this.sync = checkNotNull(sync, "sync");
> + this.listBlobsResponseToResourceList = listBlobsResponseToResourceList;
Also `checkNotNull` the `listBlobsResponseToResourceList`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7260109
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.annotation.Resource;
> +import javax.inject.Inject;
> +import javax.inject.Named;
> +import javax.inject.Singleton;
> +import java.util.Set;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author vitaly.rudenya
> + */
> +@Singleton
> +public class JoyentAsyncBlobStore extends BaseAsyncBlobStore {
> +
> + @Resource
> + protected Logger logger = Logger.CONSOLE;
Prefer initializing the logger as `Logger.NULL`. The injected implementation can be changed at runtime when configuring the desired logging module.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7259744
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
@cobracmder any member of the jclouds PMC.
You can send me an email, or send it to the dev mailing list to see if someone more familiar than me with the BlobStore apis prefers to take this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-27009721
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds-labs #524](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/524/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-26997228
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Carlos Cardenas <no...@github.com>.
Who is the main integrator / provider tester for JClouds?
I would like to chat offline about having a testing account created in the Joyent Public Cloud for this integration to move forward.
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-26999862
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.inject.Scopes;
> +import org.jclouds.blobstore.AsyncBlobStore;
> +import org.jclouds.blobstore.BlobStore;
> +import org.jclouds.blobstore.attr.ConsistencyModel;
> +import org.jclouds.encryption.bouncycastle.config.BouncyCastleCryptoModule;
> +import org.jclouds.joyent.blobstore.JoyentAsyncBlobStore;
> +import org.jclouds.joyent.blobstore.JoyentBlobStore;
> +
> +/**
> + * @author vitaly.rudenya
> + */
> +public class JoyentBlobStoreContextModule extends AbstractModule {
> +
> + @Override
> + protected void configure() {
> + install(new BouncyCastleCryptoModule());
This should better be removed from here and declared in the `defaultModules` of the api metadata.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7259675
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + ListenableFuture<Blob> getBlob(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Retrieve Blob metadata from Joyent storage
> + */
> + @Named("GetBlobMetadata")
> + @GET
> + @ResponseParser(ParseBlobMetadataFromJoyentResponse.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + @Path("/{container}/{name}")
> + public ListenableFuture<BlobMetadata> getBlobMetadata(@PathParam("container") String container,
> + @PathParam("name") String name);
> +
> + /**
> + * Create new folder in the rood directory.
Typo: root?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7241510
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #65](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/65/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30#issuecomment-26995621
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + return res;
> + }
> +
> + private static KeyPair getKeyPair(String keyPath) throws IOException {
> + BufferedReader br =
> + new BufferedReader(new InputStreamReader(JoyentBlobRequestSigner.class.getResourceAsStream(keyPath)));
> +
> + Security.addProvider(new BouncyCastleProvider());
> + PEMReader pemReader = new PEMReader(br);
> + try {
> + return (KeyPair) pemReader.readObject();
> + } finally {
> + pemReader.close();
> + }
> + }
Here you are using Bouncycastle to read the private key that is being used to sign the requests. Could you refactor the code to use the jclouds utilities, so we can remove the bouncycastle dependency? A good approach could be to configure a `PrivateKey` supplier in the Joyent guice module and use that supplier in the constructor to get the PrivateKey. Something like:
```java
@Memoized
@Provides
@Singleton
public Supplier<PrivateKey> supplyKey(@Named(JOYENT_CERT_CLASSPATH) String certClasspath,
Crypto crypto) {
return Suppliers.memoize(new Supplier<PrivateKey>() {
@Override
public PrivateKey get() {
try {
InputSupplier<InputStream> pk = Resources.newInputStreamSupplier(
Resources.getResource(certClasspath));
return crypto.rsaKeyFactory().generatePrivate(Pems.privateKeySpec(pk));
} catch (InvalidKeySpecException e) {
throw Throwables.propagate(e);
} catch (IOException e) {
throw Throwables.propagate(e);
}
}
});
}
```
And then use that supplier in the constructor to load the private key:
```java
private final PrivateKey privateKey;
@Inject
public JoyentBlobRequestSigner(@org.jclouds.location.Provider Supplier<Credentials> creds,
@Memoized Supplier<PrivateKey> privateKeySupplier,
@Named(JoyentConstants.JOYENT_CERT_FINGERPRINT) String fingerPrint)
throws IOException {
...
privateKey = privateKeySupplier.get();
...
}
```
You should be able to load the private key using the jclouds crypto utilities and remove the bouncycastle dependency.
This way you can also reuse the private key supplier wherever you need it, and you'll also have the private key cached so it is loaded only once (you can add a Suppliers.synchronize to the supplier configuration if you want to make it thread safe).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7241472
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.bouncycastle.util.encoders.Base64;
> +import org.jclouds.domain.Credentials;
> +import org.jclouds.http.HttpException;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +import org.jclouds.joyent.JoyentConstants;
> +import org.jclouds.logging.Logger;
> +
> +import javax.annotation.Resource;
> +import javax.inject.Singleton;
> +import javax.ws.rs.core.HttpHeaders;
> +import java.io.BufferedReader;
> +import java.io.IOException;
> +import java.io.InputStreamReader;
> +import java.io.UnsupportedEncodingException;
> +import java.security.*;
Avoid using wildcard imports
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7242468
Re: [jclouds-labs] Joyent Blob storage integration (#30)
Posted by Ignasi Barrera <no...@github.com>.
> + } catch (InvalidKeyException e) {
> + throw new HttpException("invalid key", e);
> + } catch (SignatureException e) {
> + throw new HttpException("invalid signature", e);
> + } catch (UnsupportedEncodingException e) {
> + throw new HttpException("invalid encoding", e);
> + }
> +
> + return res;
> + }
> +
> + private static KeyPair getKeyPair(String keyPath) throws IOException {
> + BufferedReader br =
> + new BufferedReader(new InputStreamReader(JoyentBlobRequestSigner.class.getResourceAsStream(keyPath)));
> +
> + Security.addProvider(new BouncyCastleProvider());
You should be able to remove this line if the Bouncy castle driver module has been configured.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/30/files#r7259506