Add non-dry, handle non-spec'd dbs, remove revoke all from permissions
Merge Request Checklist
-
Link to an issue #1154 (closed) #1595 (closed) #1652 (closed) -
Include the proposed fix or feature -
Include and update tests for the modified code -
Include a documentation change -
Add a CHANGELOG.md entry in the Unreleased section
Once your merge request is complete, please assign it a Meltano maintainer for review cycle. Once the review cycle finished, the reviewer shall approve the merge request so it can be merged by any Meltano maintainer.
Closes #1154 (closed)
This is (hopefully) a much more reasonable MR! This MR does 3 main things:
- Add a non-dry execution mode to meltano permissions. Now when you leave off
--dry
it will actually execute the commands on Snowflake😬 - Sets revocations to only be from databases that are in the specification
- Removes the
--full-refresh
functionality and code
Non-dry
I switched this up so that a status is passed along with each command. By default it's a None
which will print as Skipped. If the command passes it's a Success and if it fails for any reason it's an Error. I tested with
sql_grant_queries = [
{"already_granted": True, "sql": "select 1"},
{"already_granted": False, "sql": "grant usage on schema analytics.analytics to role engineer"},
{"already_granted": False, "sql": "grant scpart on schema analytics.analytics to role engineer"}
]
The following screen shot shows all the permutations of --dry
and --diff
with the status. The commands were actually executed on the server!
Non-spec'd databases
I noticed previously that when the comparisons for the spec vs what was granted on the server came back that revocations were generated for databases not defined in the spec. We have a lot of non spec'd databases because of our clones. I ran the spec several times and nothing but raw, analytics, and snowflake dbs came back!
Remove full-refresh
After going through all of the revocations for dbs, schemas, and schema objects, I came to the conclusion that the full refresh command was really useless in its implementation. It was only looking at objects defined in the spec so it wasn't good for actually handling changes (i.e. if I remove your access in the spec from a database, and then ran full refresh, no revocations would happen for you on that database!)
I think we will want to add back a full-refresh command at some point, but it'll have to be smarter than the previous implementation.
Testing
I updated the necessary tests to get this MR to pass but did not add any additional ones. This MR completes the scope of work for a data team OKR and I will work in the coming months to continue to update the test suite!