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

DefaultDynamo.java:64-64: Would be nice to migrate... #90

Closed
davvd opened this issue Jan 20, 2014 · 63 comments
Closed

DefaultDynamo.java:64-64: Would be nice to migrate... #90

davvd opened this issue Jan 20, 2014 · 63 comments

Comments

@davvd
Copy link

davvd commented Jan 20, 2014

Puzzle 1-f2c33b38 in s3auth-hosts/src/main/java/com/s3auth/hosts/DefaultDynamo.java:64-64 has to be resolved: Would be nice to migrate to jcabi-dynamo

If you have any technical questions, don't ask me, submit new tickets instead


@davvd
Copy link
Author

davvd commented Jan 20, 2014

@carlosmiranda can you please help? Keep in mind this. If you have any technical questions, don't hesitate to ask right here

@davvd
Copy link
Author

davvd commented Jan 20, 2014

@carlosmiranda The budget of this task is 30 mins. This is exactly how much time will be compensated, when the task is completed.

@carlosmiranda
Copy link
Collaborator

How do I go about doing this? Do I simply fork jcabi-dynamo, move this over there and submit a pull request for both projects?

@yegor256 yegor256 mentioned this issue Jan 21, 2014
@carlosmiranda
Copy link
Collaborator

Ah I think I misinterpreted this one. Do you mean to say that we should use jcabi-dynamo in favor of DefaultDynamo? Will DefaultDynamo be deleted when this is done?

@yegor256
Copy link
Owner

I think that DefaultDynamo class will stay, but it will not use AWS SDK any more, but jcabi-dynamo library instead. Makes sense?

@carlosmiranda
Copy link
Collaborator

Ah, all right. I'll have to read up on jcabi-dynamo then.

@carlosmiranda
Copy link
Collaborator

A few more questions regarding this one.

  1. Currently, the Dynamo interface provides a subinterface Dynamo.Client which wraps an AmazonDynamoDB instance. DefaultDynamo uses Client and invokes its get method whenever it needs to do scan or put operations. However, in Jcabi-Dynamo, it appears that the preferred idiom would be to access the table from a Region object, where the Region object now encapsulates the AmazonDynamoDB instance. Am I right in thinking that we can replace DefaultDynamo's Client field and replace it with Region?

  2. In DefaultDynamo's constructor, it creates an AmazonDynamoDB instance and includes the following code:

        final AmazonDynamoDB aws = new AmazonDynamoDBClient(
            new BasicAWSCredentials(
                Manifests.read("S3Auth-AwsDynamoKey"),
                Manifests.read("S3Auth-AwsDynamoSecret")
            )
        );
        // @checkstyle MultipleStringLiterals (1 line)
        if (Manifests.exists("S3Auth-AwsDynamoEntryPoint")) {
            aws.setEndpoint(
                Manifests.read("S3Auth-AwsDynamoEntryPoint")
            );
        }
    

    In JCabi-Dynamo, the AmazonDynamoDB is encapsulated by Region and we can specify the credentials using a Credentials instance:

    Credentials credentials = new Credentials.Simple(
        Manifests.read("S3Auth-AwsDynamoKey"),
        Manifests.read("S3Auth-AwsDynamoSecret")
    );
    Region region = new ReRegion(new Region.Simple(credentials));
    

    However, I don't see any place in the API where we can do setEndPoint. Can I just obtain the AmazonDynamoDB instance from region like below?

        if (Manifests.exists("S3Auth-AwsDynamoEntryPoint")) {
            region.aws().setEndpoint(
                Manifests.read("S3Auth-AwsDynamoEntryPoint")
            );
        }
    

Thanks in advance!

@yegor256
Copy link
Owner

  1. Yes, exactly.
  2. Everything is mutable in jcabi-dynamo, so you can't change endpoint. However, you can use Credentials.Direct decorator

@carlosmiranda
Copy link
Collaborator

From http://dynamo.jcabi.com/example-it.html, I can see that we have a mocker for Table. Do we have an equivalent for Region for the sake of unit testing?

@yegor256
Copy link
Owner

No, we don't have it, but you can submit a ticket to jcabi-dynamo and I'll implement something like it

@davvd
Copy link
Author

davvd commented Nov 19, 2014

The task description has been changed (see above). If there are concerns or objections, please give them to me right now. If not, please use the new version of the task description

@davvd
Copy link
Author

davvd commented Dec 10, 2014

@carlosmiranda it's not yours any more, because it took too long, please stop working with it

@davvd
Copy link
Author

davvd commented Dec 25, 2014

@dpisarenko please pick this up, and keep in mind these instructions. Any technical questions - ask right here

Task's budget is 30 mins (see this for explanation)

@mentiflectax
Copy link
Contributor

I have 2 questions:

  1. In com.s3auth.hosts.DefaultDynamo#load, how can I log the error?
  2. When I use JCabi-Dynamo's Region as shown below, should I do something similar to AmazonDynamoDB.shutdown after reading data from the database?
final Region region = new ReRegion(new Region.Simple(credentials));
[...] // Here I do some operations via JCabi-Dynamo

@yegor256
Copy link
Owner

@dpisarenko don't ask questions in the ticket, they won't be answered. Instead, each time you find something unclear in the code - submit a new ticket/question. Check this article: http://www.yegor256.com/2014/04/13/bugs-are-welcome.html and this one: http://www.yegor256.com/2014/11/24/principles-of-bug-tracking.html

@mentiflectax
Copy link
Contributor

@davvd @yegor256 Please look at the pull request and tell me, what needs to be changed.

@mentiflectax
Copy link
Contributor

@davvd @yegor256 I submitted a new version of the code, which can be built using mvn clean install -Pqulice. Please review.

@davvd
Copy link
Author

davvd commented Jan 2, 2015

@dpisarenko this task is in your hands for 7 days already

-5 added to your rating, at the moment it is: +160

@mentiflectax
Copy link
Contributor

@davvd Please find a reviewer for the pull request #239 .

@davvd davvd removed the @dpisarenko label Jan 5, 2015
@davvd
Copy link
Author

davvd commented Jan 5, 2015

@dpisarenko please don't touch this task, someone else will pick it up, thanks

@davvd davvd closed this as completed Jan 29, 2015
@davvd
Copy link
Author

davvd commented Jan 29, 2015

@pinaf it's not yours any more,because it took too long,please stop working with it. -60 to your rating, your total score is +255. you're working with this issue for 9 days already. do not forget to remove the TODO puzzle from the source code. -30 added to your rating, current score is: +225

@davvd
Copy link
Author

davvd commented Jan 29, 2015

my bad, should be open. I'll remove the negative rating too.

@davvd davvd reopened this Jan 29, 2015
@davvd davvd removed the @pinaf label Jan 29, 2015
@pinaf
Copy link
Contributor

pinaf commented Jan 29, 2015

@davvd OK. Keep in mind you subtracted 90 points.

@pinaf
Copy link
Contributor

pinaf commented Jan 30, 2015

@davvd can we get this fixed?

@yegor256
Copy link
Owner

yegor256 commented Feb 2, 2015

@pinaf it's fixed, I removed the points

@pinaf
Copy link
Contributor

pinaf commented Feb 3, 2015

@yegor256 points are correct but task disappeared from agenda.

@yegor256
Copy link
Owner

yegor256 commented Feb 3, 2015

@davvd assign @pinaf back to this task pls

@davvd
Copy link
Author

davvd commented Feb 3, 2015

@davvd assign @pinaf back to this task pls

@yegor256 OK @pinaf please proceed, this task is yours

@davvd davvd added the @pinaf label Feb 3, 2015
@pinaf
Copy link
Contributor

pinaf commented Feb 3, 2015

@davvd thank you. Still impeded by jcabi/jcabi-dynamo#28

@yegor256
Copy link
Owner

yegor256 commented Feb 3, 2015

@davvd we need more time here

@davvd
Copy link
Author

davvd commented Feb 4, 2015

@davvd we need more time here

@yegor256 of course, take your time

@pinaf
Copy link
Contributor

pinaf commented Feb 8, 2015

@yegor256 @davvd the issue was closed with just a test implemented. functionality still isn't there so we still need to wait for the actual implementation in a new issue.

@pinaf
Copy link
Contributor

pinaf commented Feb 21, 2015

@davvd merged. please close if alright.

@longtimeago
Copy link

@davvd close the issue, please

@pinaf
Copy link
Contributor

pinaf commented Feb 24, 2015

@davvd please close the issue man..

@pinaf
Copy link
Contributor

pinaf commented Feb 24, 2015

@davvd seriously. what is going on? why don't you close the issue?

@davvd
Copy link
Author

davvd commented Feb 24, 2015

once 90-43f9c549 puzzle is resolved (later, in another ticket), this ticket will be fully complete

@pinaf
Copy link
Contributor

pinaf commented Feb 25, 2015

@davvd close the issue

@davvd
Copy link
Author

davvd commented Feb 26, 2015

Done, thanks all, closing..

@davvd davvd closed this as completed Feb 26, 2015
@davvd
Copy link
Author

davvd commented Feb 26, 2015

@pinaf 30 mins sent to your balance (ID AP-937875029L175551X), many thanks!

+30 added to your rating, current score is: +2201

@davvd
Copy link
Author

davvd commented Jan 30, 2017

@yegor256 last puzzled solved 90-43f9c549/#292

@0pdd
Copy link
Collaborator

0pdd commented Jul 12, 2022

@davvd the puzzle #390 is still not solved.

@0pdd
Copy link
Collaborator

0pdd commented Nov 8, 2023

@davvd the only puzzle #352 is solved here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants