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

[suggestion] Support for AWS SQS GetQueueAttributes #391

Open
holtkamp opened this issue Nov 4, 2018 · 7 comments
Open

[suggestion] Support for AWS SQS GetQueueAttributes #391

holtkamp opened this issue Nov 4, 2018 · 7 comments

Comments

@holtkamp
Copy link
Contributor

holtkamp commented Nov 4, 2018

When a AWS SQS Message is fetched from a MessageQueue and is not "acknowledged" that it has been processed, it is considered to be "in flight".

To determine the number of "in flight" Messages on a MessageQueue, the GetQueueAttributes() action can be used, which also provides access to other usefull information about the MessageQueue.

The current Driver::info() could/should be used for this?

As implementation something like this can be done for the SQS Driver:

//SqsDriver.php
public function info(string $queueName = null): array
{
    $queueAttributes = [
            'prefetch' => $this->prefetch,
        ];

        if(is_string($queueName)){
            $queueUrl = $this->resolveUrl($queueName);
            $result = $this->sqs->getQueueAttributes([
                'AttributeNames' => ['All'],
                'QueueUrl' => $queueUrl,
            ]);


            $queueAttributes += $result['Attributes'] ?? [];
        }

        return $queueAttributes;
}
//AbstractQueue.php
public function info(){
    return []; //empty array for now
}
//PersistentQueue.php
public function info(){
    return $this->driver->info($this->name);
}

Would support for such a method something to be adopted in Bernard, or is it too technology-specific?

@sagikazarmark
Copy link
Contributor

Sorry for the late reply.

The thing with the Driver interface is that it's meant to be an internal thing. The public API of this library is the Queue and related interfaces.

The problem with that info method is there is no real contract for it, it's just some unstructured data. As such, I find it hard to expose it in the public API.

@holtkamp
Copy link
Contributor Author

it's just some unstructured data.

true,

but the info() method is barely / not used in the complete library, right? And this is exactly a use case where it can come in handy: to get some additional information about a specific queue...

@sagikazarmark
Copy link
Contributor

The original purpose of that method was to be used in Juno with some supported drivers, but the library evolved a lot and that use case is really not a priority one. I'm actually thinking of removing that method.

If I were you I would probably write my own layer of abstraction to get queue information in my application. There is nothing wrong with keeping the responsibilities separated.

@henrikbjorn
Copy link
Contributor

I would be against removing info. Also it can't be done without a major version release.

@sagikazarmark
Copy link
Contributor

A major version release is imminent anyway. We are not even at 1.0 yet.

What are the reasons to keep info? Right now it kinda goes against the idea of keeping the Driver an internal concept to the library.

But even if we keep it around, it's still an internal thing and I think it should stay that way, otherwise it's a leaky abstraction.

@henrikbjorn
Copy link
Contributor

The reason for not moving it, is that i am using it and i think it is vital to have a way of getting information about the queues. If there was a better abstraction or something, that did this better then i am all for removing it.

@sagikazarmark
Copy link
Contributor

What kind of information are we talking about? Can we define a common set of attributes that we want to expose?

Right now I don't see that possible and I think that info retrieval will always be a driver/service dependent thing which is probably specific to your application somehow. As such, from an architectural perspective this should probably be a separate responsibility from messaging.

My concern is that if we agree in a common set of attributes it won't deliver the value we expect it to do and we will be pushed to change things back this unstructured array thing.

I'm okay keeping it if we say that at no point do we take responsibility for anything returned from that method. It's an unstructured map of data and we keep the right to break it anytime we see it fit. But that's probably as bad as having not enough, not useful information.

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

No branches or pull requests

3 participants