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

Fix for --format provider-json when run with google-batch provider #297

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pgm
Copy link

@pgm pgm commented Aug 30, 2024

A fix for a very minor issue where running dsub --provider google-batch ... --format provider-json was resulting in the following exception:

Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/miniconda/base/envs/dsub/bin/dstat", line 33, in <module>
    sys.exit(load_entry_point('dsub', 'console_scripts', 'dstat')())
  File "/Users/pmontgom/dev/dsub/dsub/commands/dstat.py", line 224, in main
    formatter.prepare_and_print_table(rows, args.summary)
  File "/Users/pmontgom/dev/dsub/dsub/lib/output_formatter.py", line 80, in prepare_and_print_table
    row = self.prepare_output(row)
  File "/Users/pmontgom/dev/dsub/dsub/lib/output_formatter.py", line 60, in prepare_output
    if col in row:
  File "/opt/homebrew/Caskroom/miniconda/base/envs/dsub/lib/python3.10/site-packages/proto/message.py", line 688, in __contains__
    pb_value = getattr(self._pb, key)
AttributeError: last-update

Tried to create a fix, which not particularly elegant, makes minimal changes.

Happy to take feedback if you think it'd be better fixed a different way.

Thanks.

@pgm pgm changed the title Fix for --format provider-json Fix for --format provider-json when run with google-batch provider Aug 30, 2024
@wnojopra
Copy link
Contributor

Thanks (again!) for the PR, @pgm! Yea, this particular dstat format has been neglected so far for the google-batch provider. It's great that you've found a quick fix for it. I myself have been working on finishing dstat implementation for Batch and getting the e2e_dstat.sh test passing.

And then a few points on dsub's current PR acceptance policy:

  • First and most importantly, thank you for the PR. We very much appreciate community support.
  • Currently, the "source of truth" for dsub source code is in an internal repo. We won't be able to merge this PR directly. What we can do however is take it, add it to our internal system, and then acknowledge your contributions when it is shared back out to this repo.
  • We are looking into ways to update our contribution acceptance policy so that we can directly merge PRs from the community.

@wnojopra
Copy link
Contributor

wnojopra commented Sep 4, 2024

Hi @pgm!

As mentioned in our CONTRIBUTING doc, contributions to dsub must be accompanied by a Contributor License Agreement. Please review and sign our Contributor License Agreement and send it to our IP team at [email protected].

Thanks!

@pgm
Copy link
Author

pgm commented Sep 4, 2024 via email

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.

2 participants