initial new stream with integration logs

[Volna13]

Goal: Add new stream with integration logs
Link to get IntegrationLogs: "https://slack.com/api/team.integrationLogs"

Response example:
{ "ok": true, "logs": [ { "user_id": "U0SSSSSV8A", "user_name": "username", "date": "1665778704", "change_type": "added", "app_type": "Stitch Test", "app_id": "AAAA4AAAA" }, { "user_id": "U03SSSSSA", "user_name": "username", "date": "1665777146", "change_type": "added", "app_type": "Slack Tooling Tokens Vendor", "app_id": "AAA4AAAAA", "scope": "identify,app_configurations:read,app_configurations:write" } ... ], "paging": { "count": 100, "total": 13, "page": 1, "pages": 1 } }

[pnadolny13]

@Volna13 responding to https://meltano.slack.com/archives/CMN8HELB0/p1668691051252529?thread_ts=1668527444.888179&cid=CMN8HELB0 from slack here.

In terms of pagination currently all stream classes inherit from SlackStream that defines next_page_token_jsonpath which is used in the SDK's get_next_page_token method. The paging info for those streams look like:

    "response_metadata": {
        "next_cursor": "dGVhbTpDMDYxRkE1UEI="
    }

And youre new one for team.integrationLogs is different like:

"paging": {
	"count": 3,
	"total": 3,
	"page": 1,
	"pages": 1
}

You can override the get_next_page_token and get_url_params in your IntegrationLogsStream class. You'd probably want to pull out the page number from the paging key and if its smaller than pages then return page +1 otherwise return None to stop the iteration cycle (i.e. no more pages). Also you'll need to override get_url_params since you need to pass in the page number to the request if its available.

Another thing to consider is that this stream requires higher level credentials (admin access) than the rest of the stream. We probably need to consider how to exclude this stream by default since auth/discovery for current users will fail.

Does that makes sense? You can post in the slack channel #singer-tap-development if you need more help.

@edgarrmondragon did I get that right? What do you think about the possibility of excluding this new stream by default since it requires admin privileges that most people wont have, how would we do that? We could also use stream selection in the hub definition that excludes it if needed 🤔 . We may want to get #13 in before this to bump the sdk version.

[edgarrmondragon]

@edgarrmondragon did I get that right?

@pnadolny13 Yup, a new implementation of get_next_page_token or a new pagination helper class is needed for the new stream.

What do you think about the possibility of excluding this new stream by default since it requires admin privileges that most people wont have, how would we do that? We could also use stream selection in the hub definition that excludes it if needed 🤔 . We may want to get #13 in before this to bump the sdk version.

The Singer catalog spec has a selected_by_default flag that we are not using for discovery at the moment, but we could with a quick change to MetadataMapping.get_standard_metadata. The way to do it without that would be to override metadata in the stream:

class UnselectedStream(Stream):

    @property
    def metadata(self):
        meta = super().metadata
        # Deselect
        if self._tap_input_catalog is None:
            meta.root.selected = False
        return meta

That should work but I'm not a fan of 😅.

I've logged meltano/sdk#1203

[Volna13]

@pnadolny13

In terms of pagination currently all stream classes ....

Thanks for the explanation about pagination. I will try to implement and add to this PR.

Another thing to consider is that this stream requires higher level credentials (admin access) than the rest of the stream. We probably need to consider how to exclude this stream by default since auth/discovery for current users will fail.

Regarding the fact that this should not be the default stream, I agree with you, I would be grateful if you could tell me how I can make this stream optional? (perhaps in order for it to work, we could somehow make it mandatory to be specified explicitly in the select)

[Volna13]

@pnadolny13 can you check my last commit?

[pnadolny13]

@Volna13 this looks good to me! I created meltano/hub#1055 to exclude this admin stream by default in MeltanoHub which I think would work vs Edgar's solution in #14 (comment).

@edgarrmondragon would you mind reviewing this PR also. What do you think about the hub select criteria idea for excluding this stream by default?

[pnadolny13]

@Volna13 after getting some feedback from others it sounds like the best path forward for disabling this stream by default is to add a new config option as AJ describes in meltano/hub#1055 (comment).

I think what that means is that we need to add an if block to

tap-slack/tap_slack/tap.py

Line 73 in 2c932e3

def discover_streams(self) -> List[Stream]:

where it checks if an include_admin_streams config option is set to true before adding the integration_log stream to the list.

[pnadolny13]

@Volna13 I created #19 as a mechanism to exclude admin streams. Once that merges you can pull those changes into this branch and append your stream to the admin list.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[pnadolny13]

@Volna13 I just merged #19 so if you pull those changes into your branch you should be able to use that admin stream mechanism in this PR.

[pnadolny13]

@Volna13 how is it going? Let us know when youre ready for a re-review.

[Volna13]

how is it going? Let us know when youre ready for a re-review.

@pnadolny13 noticed that I did not take into account the fact that I did not add a way to replicate data for a new stream. Now I've switched to learning the orchestrator, but within a few days, I think I'll be done.

I understand correctly that all the updates that we talked about above are already merged into this repository and I just need to update my repository and add a way to replicate data

[tayloramurphy]

@Volna13 are you still looking to get this PR ready to merge?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information