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

Add user_id,group_id,tenant_id to EventStream. #94

Merged
merged 1 commit into from Oct 27, 2017

Conversation

lfu
Copy link
Member

@lfu lfu commented Oct 16, 2017

Add user_id,group_id,tenant_id to EventStream.

User wants to get the event initiator from MiqEvent and uses it in automate code.
Blocks ManageIQ/manageiq#16179.

https://bugzilla.redhat.com/show_bug.cgi?id=1487749

cc @kbrock @Fryguy @mkanoor

@miq-bot
Copy link
Member

miq-bot commented Oct 16, 2017

Checked commit lfu@28ed745 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@lfu
Copy link
Member Author

lfu commented Oct 16, 2017

@miq-bot add_label enhancement

@mkanoor
Copy link
Contributor

mkanoor commented Oct 18, 2017

👍

@kbrock
Copy link
Member

kbrock commented Oct 20, 2017

Do wish we did not have user_id in event - and wish it could be derived in the receiver (since an event from vmware would not actually have a user_id on it)

But, this seems like a good way to make sure that a user is passed through the system. (rather than the bogus "system" phrase)

👍

@Fryguy
Copy link
Member

Fryguy commented Oct 23, 2017

My concern here is that storing user_ids typically has complications for the global region and we usually store the user "name" in those cases. However, I don't think event_streams are replicated, so maybe it doesn't matter

cc @gtanzillo @gmcculloug Thoughts?

@lfu
Copy link
Member Author

lfu commented Oct 24, 2017

@Fryguy In my understanding user_id is unique within and across the region while userid is unique within the region but not across the region. So user_id seems a better choice than userid. What do I miss?

@Fryguy
Copy link
Member

Fryguy commented Oct 24, 2017

The problem is that "jsmith" in region 1 will have an id of 1_000_000_000_001, but in region 99 his id will be 99_000_000_000_001. If we replicate region 1 into region 99, then his name exists twice, and then you might need to merge his events from both regions. If you link by id only, then it can only see his events from a particular region, while if you link by userid, then you can see all of them. Since we do replicate events (they're not in the default_replication_exclude_tables.yml), I want @gtanzillo and @gmcculloug to approve first before merge.

@gtanzillo
Copy link
Member

I'm good with this. Discussing with @gmcculloug, automate needs to reference the actual user of the event and not necessarily the user with the same name in the global region. Also, event_streams is replicated. But, we can always use the user_id to look up the actual user and the find the one with the same username in the global if necessary. We've done that in other places.

@gmcculloug
Copy link
Member

@Fryguy Please review/merge.

@Fryguy Fryguy merged commit 6d17aad into ManageIQ:master Oct 27, 2017
@Fryguy Fryguy added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 27, 2017
@Fryguy Fryguy added the schema label Oct 27, 2017
@lfu lfu deleted the miq_event_user_1487749 branch September 29, 2018 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants