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

Util: Potentially fragile fix for key errors in make_timeline #1541

Merged
merged 3 commits into from Jul 10, 2020
Merged

Util: Potentially fragile fix for key errors in make_timeline #1541

merged 3 commits into from Jul 10, 2020

Conversation

JLGarber
Copy link
Collaborator

@JLGarber JLGarber commented Jul 5, 2020

(This should resolve #1538)

This isn't the cleanest way to do it, but for the interim, until we update make_timeline to use V2 of the API, I think this is close enough. I ran a number of different encounters through it, specifically including the 3 failing ones I mentioned in the linked issue, and all of them went off successfully.

(I call this fix fragile because it just assumes that any events that are missing sourceID will include event.source.id and event.source.name. This is how it seems to work, but it is an assumption. I poked at explicitly checking for these elements a little better, but I didn't get anywhere.)

@JLGarber
Copy link
Collaborator Author

JLGarber commented Jul 5, 2020

Mmph. @panicstevenson, sorry to tag you in here, but I'm out of my depth here. The closest thing I could find was from two years ago, but I'm not really sure whether this is a problem on my end or in the CI configuration.

@panicstevenson
Copy link
Contributor

panicstevenson commented Jul 5, 2020

Looks to be a CI issue, specifically pylint for 2.2.X has a dependency on isort that can take on any version above 4.2.5. isort released a new version recently, which appears to have breaking changes in pylint. This is probably obvious, since newer versions of pylint have capped the version of isort to anything less than 5.X and there is an open issue denoting the same. I'm assuming that your system (as with mine) already has this requirement for isort established and has no reason to go out and grab a more recent version; however, it looks like on a fresh-install this is broken.

EDIT:

This is easily replicated as assumed above:
Uninstall python packages:

pip freeze | xargs pip uninstall -y

Install a pinned version of pylint:

python -m pip install pylint==2.2.3 --user

Run pylint:

$ python -m pylint util/make_timeline.py --errors-only
Traceback (most recent call last):
  File "C:\Program Files\Python37\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Program Files\Python37\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\__main__.py", line 8, in <module>
    pylint.run_pylint()
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\__init__.py", line 20, in run_pylint
    Run(sys.argv[1:])
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\lint.py", line 1608, in __init__
    linter.check(args)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\lint.py", line 938, in check
    self._do_check(files_or_modules)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\lint.py", line 1071, in _do_check
    self.check_astroid_module(ast_node, walker, rawcheckers, tokencheckers)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\lint.py", line 1154, in check_astroid_module
    walker.walk(ast_node)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\utils.py", line 1271, in walk
    cb(astroid)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\checkers\imports.py", line 507, in leave_module
    std_imports, ext_imports, loc_imports = self._check_imports_order(node)
  File "C:\Users\Panic\AppData\Roaming\Python\Python37\site-packages\pylint\checkers\imports.py", line 664, in _check_imports_order
    isort_obj = isort.SortImports(
AttributeError: module 'isort' has no attribute 'SortImports'

@JLGarber
Copy link
Collaborator Author

JLGarber commented Jul 6, 2020

Thanks for looking into that. Glad it's a relatively simple fix for now.

@quisquous
Copy link
Owner

I think if you merge in the latest changes from master, these will now pass.

@panicstevenson
Copy link
Contributor

🎉

@JLGarber
Copy link
Collaborator Author

Wonderful. Glad that's sorted.

@quisquous quisquous merged commit aedb801 into quisquous:master Jul 10, 2020
@quisquous
Copy link
Owner

Thanks for your patience!!!

@JLGarber JLGarber deleted the make-timeline-fflogs-key-error-fixes branch July 11, 2020 19:07
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.

Util: Make_timeline key failures when generating from FFLogs
3 participants