Fixing groups in config file not considered for group migration job

[william-conti]

Changes

If the customer runs the assessment, then changes the configuration file to add a new group, UCX won't be able to reflect the changes.
This change fixes the above scenario and aims towards an incremental group migration.

Linked issues

Closes #941

Tests

• manually tested
• added unit tests

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f951aac) 87.76% compared to head (1612907) 87.78%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #943      +/-   ##
==========================================
+ Coverage   87.76%   87.78%   +0.02%     
==========================================
  Files          43       43              
  Lines        5213     5223      +10     
  Branches      939      943       +4     
==========================================
+ Hits         4575     4585      +10     
  Misses        434      434              
  Partials      204      204              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mwojtyczka]

I am not sure this is the best way to solve the issue. It would be better to filter the groups when we fetch them (in _fetcher method). We are already filtering the groups when we crawl. We just need to add simple filter when we fetch as well. This would be one-liner fix.

[william-conti]

@mwojtyczka I would like to identify groups specified in the conf that aren't in the groups table. Today if you specify random groups in the conf it is completely silented.

[mwojtyczka]

@mwojtyczka I would like to identify groups specified in the conf that aren't in the groups table. Today if you specify random groups in the conf it is completely silented.

Got it. It's another use case then. There is another issue with the fetcher that is not filtering the groups from the config.

[william-conti]

@mwojtyczka After discussion with Jince, it's better to move the logic to the fetcher() because of what you mention and also for keeping the consistency of the logic. Let's keep things as clean as possible

[mwojtyczka]

Do we need this detailed log? If you look at get_filtered_groups method that we use for crawler, we just filter out the groups based on _include_group_names. If you report issues with _include_group_names here then we should also do it in get_filtered_groups method to make it consistent.

Can we eliminate all the additional loops?

shouldn't something like below be enough?

for row in self._backend.fetch(f"SELECT * FROM {self._full_name}"):
  g = MigratedGroup(*row)
  if not self._include_group_names or g.name_in_workspace in self._include_group_names:
      yield g