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.

Sign up for GitHub

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

Jump to bottom

Graql shell crashes if a user doesn't have a home directory #7

Closed
grabl opened this issue Sep 21, 2018 · 5 comments · Fixed by #16
Closed

Graql shell crashes if a user doesn't have a home directory #7

grabl opened this issue Sep 21, 2018 · 5 comments · Fixed by #16
Assignees
@vmax
Labels
status: solved

Comments

@grabl
Copy link
Contributor

grabl commented Sep 21, 2018

This issue was originally posted by @kasper-piskorski on 2018-01-26 12:11.

NB: not sure if we should fix that, just logging it here so that we know

 java.io.IOException: No such file or directory at java.io.UnixFileSystem.createFileExclusively(Native Method) at java.io.File.createNewFile(File.java:1012) at ai.grakn.graql.GraqlShell.setupHistory(GraqlShell.java:474) at ai.grakn.graql.GraqlShell.executeRepl(GraqlShell.java:394) at ai.grakn.graql.GraqlShell.start(GraqlShell.java:370) at ai.grakn.graql.GraqlShell.runShell(GraqlShell.java:266) at ai.grakn.graql.GraqlShell.runShell(GraqlShell.java:170) at ai.grakn.graql.GraqlShell.main(GraqlShell.java:165) at ai.grakn.dist.DistGraql.run(DistGraql.java:52) at ai.grakn.dist.DistGraql.main(DistGraql.java:44)

I believe the cause is this:

 private static final String HISTORY_FILENAME = StandardSystemProperty.USER_HOME.value() + "/.graql-history";

If a user doesn't have a home directory it throws an exception on file creation.
@grabl
Copy link
Contributor Author

grabl commented Sep 21, 2018

This comment was originally posted by @haikalpribadi on 2018-04-14 18:13:48+02:00.

I think we should catch this exception and tell the user they need to fix it (through an error message). @lolski @kasper-piskorski

@grabl
Copy link
Contributor Author

grabl commented Sep 21, 2018

This comment was originally posted by @lolski on 2018-04-14 23:59:36+02:00.

Agreed @haikalpribadi

@grabl
Copy link
Contributor Author

grabl commented Sep 21, 2018

This comment was originally posted by @kasper-piskorski on 2018-04-16 18:02:03+02:00.

@lolski @haikalpribadi tell user to fix it? what's wrong with having no home directory? shouldn't we instead make the history file optional?

@grabl
Copy link
Contributor Author

grabl commented Sep 21, 2018

This comment was originally posted by @haikalpribadi on 2018-04-27 23:54:15+02:00.

@kasper-piskorski If we don't need to fix it, I don't have a problem with that, Kasper. But is it possible to make the HISTORY_FILENAME optional?
@lolski

@grabl
Copy link
Contributor Author

grabl commented Sep 21, 2018

This comment was originally posted by @lolski on 2018-05-08 11:33:14+02:00.

Yes, it should be possible. Shall I create a ticket for that?

@vmax vmax transferred this issue from vaticle/typedb Oct 7, 2019
@grabl grabl mentioned this issue Oct 7, 2019
Closed
@vmax vmax mentioned this issue Nov 8, 2019
Merged
@vmax vmax assigned vmax and unassigned kasper-piskorski Nov 8, 2019
@vmax vmax closed this as completed in #16 Nov 8, 2019
vmax added a commit that referenced this issue Nov 8, 2019
@vmax
Make history file optional (#16)
19fe1b2 
## What is the goal of this PR?

Fix #7
Previously, Grakn Console would crash in case history file could not be created (possible reasons are user's home directory being non-present or mounted read-only)

## What are the changes implemented in this PR?

- Bump `@graknlabs_protocol` version
- Make an adapter class which allows to `.flush()` history object regardless of whether it's backed by `FileHistory` or `MemoryHistory`
- Resort to using in-memory history in case history file could not be created
lolski pushed a commit to lolski/typedb-console that referenced this issue Jun 9, 2021
@vmax
Updates client-nodejs to use latest SessionService gRPC API (vaticle#7)
af9a565 
# Why is this PR needed?

Fixes vaticle#5 
So we are able to use `client-nodejs` with the latest `@graknlabs_grakn_core`

# What does the PR do?

- Upgrades commit hash of `@graknlabs_grakn_core`
- Fixes the client to comply with new API

# Does it break backwards compatibility?

—

# List of future improvements not on this PR

—
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vmax vmax

Labels
status: solved
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make history file optional
3 participants
@vmax @kasper-piskorski @grabl