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

Adding foreign code extractors for spark sql and exporting required classes #980

Merged
merged 8 commits into from
Sep 20, 2023

Conversation

skbitsp
Copy link

@skbitsp skbitsp commented Sep 13, 2023

Adding foreign code extractors for spark sql and exporting required classes

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch skbitsp/jupyterlab-lsp/sk-lsp

@krassowski
Copy link
Member

@skbitsp can you please rebase to trigger the CI checks which were re-enabled on the 4.x branch by #982?

@skbitsp
Copy link
Author

skbitsp commented Sep 15, 2023

@skbitsp can you please rebase to trigger the CI checks which were re-enabled on the 4.x branch by #982?

Done ✅.

@skbitsp
Copy link
Author

skbitsp commented Sep 17, 2023

Hi, actually our team is working on a timeframed project, merging this PR would really help us, so could I please get an estimated timeframe of when it would be merged?

@krassowski
Copy link
Member

Merging itself is not a problem, but since the tests are failing it would take some manual checks to do a release. You can help by fixing up CI / jstest/lint which needs simple fixes on typing in Python. Otherwise I will merge when I get to backporting the CI fixes after releasing a new version.

@skbitsp
Copy link
Author

skbitsp commented Sep 18, 2023

Okay, I was checking the jstest/lint test, it fails at this: "python_packages/jupyter_lsp/jupyter_lsp/manager.py:64: error: Incompatible types in assignment (expression has type "Bool[bool, Union[bool, int]]", variable has type "bool") [assignment]"

This PR doesn't make any changes to that file, so not sure how to fix it without changing the existing code

@skbitsp
Copy link
Author

skbitsp commented Sep 19, 2023

If this will take time, I can also explore taking a clone of the repo and making a custom deployment for our company's internal usage. But every MP can only publish one version. Since this one has server extension as well as Jupyter lab extension, and there's some code duplication also amongst the two extensions.. is there a clear segregation bw the code so that we can create multiple mps?

@krassowski
Copy link
Member

This PR doesn't make any changes to that file, so not sure how to fix it without changing the existing code

I pushed a few commits, but if you are asking for an ASAP release of an older version, it would help if you could contribute towards reducing the maintenance burden (we all like puppies) and yes it involves changing existing code.

@krassowski
Copy link
Member

Now the lint job is failing only on the code that you did modify ;)

$ prettier --check "**/*{.ts,.tsx,.js,.jsx,.css,.json,.md,.yml}"
Checking formatting...
[warn] packages/jupyterlab-lsp/src/index.ts
[warn] Code style issues found in the above file. Forgot to run Prettier?

@skbitsp
Copy link
Author

skbitsp commented Sep 20, 2023

oops ;)
Prettified all the problematic files now

@skbitsp
Copy link
Author

skbitsp commented Sep 20, 2023

All the prettify checks pass on my machine now:

prettier --check "**/*{.ts,.tsx,.js,.jsx,.css,.json,.md,.yml}"
Checking formatting...
All matched files use Prettier code style!

@krassowski
Copy link
Member

It seems you have a different version of prettier installed or different settings are being picked up. Force-pushing after discarding df14ca2 commit should be enough to make CI pass.

@skbitsp
Copy link
Author

skbitsp commented Sep 20, 2023

Done ✅

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you @skbitsp!

@krassowski krassowski merged commit a4d9e95 into jupyter-lsp:4.x Sep 20, 2023
22 checks passed
@skbitsp
Copy link
Author

skbitsp commented Sep 21, 2023

Thank you so much for merging :)
Will this automatically produce a release version or are there any additional steps?

@krassowski
Copy link
Member

v4.3.0 is out on PyPI and on npm. The next step would be forward-porting the spark sql extractors (opening a PR against main branch) if you wish them to be included in future releases (5.0+) too.

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