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

gscan: new -a and -o OWNER-PATTERN options #2096

Merged
merged 1 commit into from
Jan 10, 2017
Merged

Conversation

matthewrmshin
Copy link
Contributor

@matthewrmshin matthewrmshin commented Jan 3, 2017

Can now filter by suite owners.
Display host column by default, if scanning multiple hosts.
New owner column, display by default if scanning other users.
Refactor:

  • Results dict key now a tuple of (host, owner, suite) - as multiple
    users can run suites with the same names on the same hosts.
  • Remove unnecessary logic, e.g. pointless class inheritance, multiple caching of previous results, etc.
  • Can now use the data structure returned by port scan without fiddling.
  • Use cylc ls-checkpoint instead of cylc cat-state to get info for
    a stopped suite.
  • Moved shared functions between cylc.gui.gpanel and cylc.gui.gscan
    to a separate module cylc.gui.scanutil.

Close #1625.

@matthewrmshin matthewrmshin added this to the next release milestone Jan 3, 2017
@matthewrmshin matthewrmshin self-assigned this Jan 3, 2017
@matthewrmshin matthewrmshin force-pushed the gscan branch 2 times, most recently from 98eb821 to 3c13d4f Compare January 4, 2017 10:02
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine, unfortunately I've found a few bugs:

  1. The groups feature is broken
  2. User defined default columns feature is broken
  3. Launching gcylc works via the right-click menu but not on double click
  4. If a host is removed (via the right click menu) any suites running on that host show as stopped (rather than disappearing as they did before)
  5. gpanel would appear to be running in -o .* mode!
  6. Changing hosts (via the right click menu) isn't working as expected for me

@matthewrmshin
Copy link
Contributor Author

@oliver-sanders reported issues should now be fixed.

@oliver-sanders
Copy link
Member

@matthewrmshin Bugs tested as fixed. Just a couple of small niggles left:

  • The tree collapses on update
  • The group mode is now displaying (ungrouped) at all levels of the tree (see before and after screenshots)

screenshot

Can now filter by suite owners.
Display host column by default, if scanning multiple hosts.
New owner column, display by default if scanning other users.
Refactor:
* Results dict key now a tuple of (host, owner, suite) - as multiple
  users can run suites with the same names on the same hosts.
* Reduce unnecessary logic and caching of previous results.
* Can now use the data structure returned by port scan without fiddling.
* Use `cylc ls-checkpoint` instead of `cylc cat-state` to get info for
  a stopped suite.
* Moved shared functions between `cylc.gui.gpanel` and `cylc.gui.gscan`
  to a separate module `cylc.gui.scanutil`.
@matthewrmshin
Copy link
Contributor Author

@oliver-sanders reported small niggles should now be fixed.

Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Review in progress, looks good so far...)

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good.

@hjoliver hjoliver merged commit 28efd20 into cylc:master Jan 10, 2017
@matthewrmshin matthewrmshin deleted the gscan branch January 10, 2017 06:51
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

Successfully merging this pull request may close these issues.

3 participants