Comments (13)
I think that this is nice but consider it in the level of a syntactic sugar. I don't think an extra reference to Storage service would be a problem.
Two more things to consider are:
- consistency with the Datastore API
- This would either make the blobs/bucket mutable or operation would have to return a new copy
from google-cloud-java.
I am taking over on this.
It would be nice for functional Bucket
to return functional Blob
objects (in methods like get
or list
) rather than BucketInfo
objects (so that users do not need to do further conversions as new Blob(bucket.get("someBlob"))
).
This is perfectly doable for get
methods. For list
method instead we use the underlying storage.list
wich returns a ListResult<BlobInfo>
. ListResult
is an iterable that handles pagination and is also serializable. ListResult
therefore takes only seriablizable types. Because of that we can not instantiate ListResult<Blob>
. I see two solutions here given that we want to preserve the fact that storage works with serializable types (and I think we should):
- Define a BlobList class which wraps ListResult and is an
Iterable<Blob>
- Let bucket methods return
BlobInfo
rather than functionalBlob
I would personally prefer solution 1.
@jgeewax @aozarov @ajkannan thoughs?
from google-cloud-java.
I also think that Bucket should return (functional) Blob.
Another option would be remove the constrain that T
needs to be Serializable
but
document that the results
param must be Serializable
. and then pass results<Blob>
that keeps a transient reference to storageService and convert BlobInfo
to Blob
on the fly.
Now the question is how do we create storageService upon de-serialization of that implementation
of results. For that I think we can do the following (probably should be done as a prior PR):
I think it is reasonable to ask XXXFactory
to be serialize.
with bothXXXOptions
and XXXFactory
serializable we can always construct the implementation
for XXX service.
We could pass both factory and option to the service impl (maybe even have part of the service contract) getFactory
as we have now getOptions
.
If we do so, we could technically even make the functional Blob
and Bucket
Serializable
and the
keep the signature of ListResult
as is...
What do you think?
from google-cloud-java.
I am not sure I would want functional classes as Blob, Bucket or XXX Service to be serializable. I think having serializable BlobInfo and BucketInfo is good as the classes are semantically containers or metadata. But for XXX Service, Blob and Bucker I struggle to find reasons for making them serializable.
from google-cloud-java.
Yes, we don't have to make or declare them as serializable, the other approach was to transform them on the fly and remove the requirement of having T serializable. Any opinion on that?
from google-cloud-java.
(Sorry, closed by mistake)
The other approach would still require making XXXFactory
serializable, right? Removing the requirement of having T serializable but keeping a serializable ListResult
seems a bit... unusual :) to me.
from google-cloud-java.
Yes, it would still need to make XXXFactory
serializable. I think that is reasonable as the factory usually
embeds the logic of how to construct the service (or fetch it from some place).
It is actually very common to have a serialize container but not to force its elements compile time
signature to be serialize. Lots of examples from the JDK, e.g. ArrayList.
from google-cloud-java.
Oh I see!
I have already seen serializable factories so that's ok. The only thing left that might be considered "strange" is having a getFactory
method in the XXX Service implementation (or contract). Do you think it's ok?
from google-cloud-java.
I don't see a problem with that, but I don't think it is a must in this case.
We can explicitly pass factory to Bucket
and Blob
if you prefer.
from google-cloud-java.
No it's ok then. I prefer having a getFactory method in XXX Service rather then passing the factory to Blob and Bucket. I'll work on a PR that does this before moving back Blob and Bucket.
from google-cloud-java.
Coming back to this after some thinking.
To avoid having to deal with a lot of Serializable
but still reuse ListResult
we could split ListResult
into its interface (ListResult
) and implementation (BaseListResult
). We will still have a serializable BaseListResult
class for BucketInfo
and BlobInfo
used by StorageImpl
. At the same time we can define a new class BlobListResult
, implementing ListResult
on the Blob
type, and used internally by Bucket
. By doing this we have that both Bucket
and Storage
return ListResult
, BaseListResult
is the original ListResult
class almost unchanged and we are free to make BlobListResult
serializable if need be. Thoughts?
PS: sorry for being a bit annoying on this but I feel like we should close this issue for good :)
from google-cloud-java.
@mziccard I think that is a fine plan. We may later not need BlobListResult but you should not be blocked by it.
from google-cloud-java.
Fixed by #171
from google-cloud-java.
Related Issues (20)
- Add guides for the best practices of client lifecyle
- Adopt Java Platform Module System (JPMS)
- VertexAI Nightly Failure: RESOURCE_EXHAUSTED
- ITChatSessionIntegrationTest failed with IllegalStateException (flaky)
- Getting null in getAccessToken after giving Service Account Secrets Json HOT 3
- Option classes do not support JSON serialization/deserialization
- The example does not set the FieldMask property that is required to make the call... HOT 1
- Update RetrySettings guide to use the java.time methods
- Nightly Build Failure - DialogFlow ITSystemTest
- Nightly Flake: Dataproc listWorkflowTemplateTest
- Vertex AI Generative model: Add support for context cache HOT 1
- Do I need to enable the Vertex AI Search for Retail API if I want to use ProductServiceSettings. newBuilder()? HOT 1
- Nightly Flake: VertexAI RESOURCE_EXHAUSTED HOT 4
- Nightly Flake: VertexAI - Read Timeout HOT 2
- [Translate] Warning when using ADC + API Key HOT 7
- Nightly flake: dialogflow.cx.v3beta1.it.ITSystemTest
- Nightly flake: grpc generics compilation failure HOT 1
- Compute API: ConfidentialInstanceType missing from ConfidentialInstanceConfig HOT 2
- Nightly flake: Compute - Overly prescriptive assertion
- Nightly flake: Vertex AI: ITChatSessionIntegrationTest:sendMessageWithNewConfigs_historyContainsFullConversation HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from google-cloud-java.